Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(734)

Issue 581770043: 5824 cleanup of output-distance (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months ago by hanwenn
Modified:
3 months, 3 weeks ago
Reviewers:
Dan Eble, hahnjo
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

commit 37c6ba676f4722a4e5a7b9965b64d46eff5e0bb1 (output-distance-test) Author: Han-Wen Nienhuys <hanwen@lilypond.org> Date: Fri Mar 6 16:47:26 2020 +0100 Fix test target name in python/GNUmakefile commit 79a1f4de2ac6035bbedf0e49841581d21533d3b7 Author: Han-Wen Nienhuys <hanwen@lilypond.org> Date: Fri Mar 6 16:32:12 2020 +0100 scripts/build: run self-test as part of 'check' suite. commit 613d8478797784ecc9e7676639375e7f18d4abf1 Author: Han-Wen Nienhuys <hanwen@lilypond.org> Date: Fri Mar 6 16:30:23 2020 +0100 output-distance: avoid calling strip() on None commit 80079c73090bdd7608567c76508bc1c9f1c83d33 Author: Han-Wen Nienhuys <hanwen@lilypond.org> Date: Fri Mar 6 11:54:12 2020 +0100 output-distance: make self test pass again: * Remove --create-images option. This is always true. * Remove --no-compare-images option. We always compare. * Drop dynamic table headings. before/after also works * Use same options as regtest in self-test. * Check that .pngs/.jpgs are created. commit 973677710c4f7b4551a0283f1084dfa68c4357d0 Author: Han-Wen Nienhuys <hanwen@lilypond.org> Date: Fri Mar 6 11:53:53 2020 +0100 output-distance: remove support for stencil expression checks This must have been broken since commit c1a4e44 "don't do expression comparison." (Jun 2007). This also caused a self-test failure in output-distance, which means it hasn't been run for 13 years. Adapt for expression/origin removal. Make test_compare_signatures pass, commit 1918da8bd677dd7ec2efd4bc1280002d20a65fce Author: Han-Wen Nienhuys <hanwen@lilypond.org> Date: Fri Mar 6 10:40:27 2020 +0100 output-distance: cleanups * Centralize imports at top * Use doc strings rather than comments * Add more comments/docstrings * Remove unused GrobSignature.origin * Clarify method names: * snippet_fn_re as TextFileCompareLink instance variable * use time.time() rather than time.clock() * use log_verbose() rather than print() * set the actual centroid in GrobSignature. * remove unused axis_centroid() method

Patch Set 1 #

Patch Set 2 : output-distance refactor #

Patch Set 3 : make doc fix #

Total comments: 4

Patch Set 4 : drop python/ fix #

Patch Set 5 : backward compatibility; verify that make-check actually works #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+179 lines, -261 lines) Patch
M GNUmakefile.in View 1 2 1 chunk +1 line, -1 line 0 comments Download
M scm/stencil.scm View 5 chunks +8 lines, -57 lines 0 comments Download
M scripts/build/GNUmakefile View 1 2 3 4 2 chunks +5 lines, -0 lines 1 comment Download
M scripts/build/output-distance.py View 1 2 3 4 53 chunks +165 lines, -203 lines 0 comments Download

Messages

Total messages: 18
hanwenn
output-distance refactor
4 months ago (2020-03-06 16:13:29 UTC) #1
Dan Eble
The title of the review is "5824 Fix test target name in python/GNUmakefile" but the ...
4 months ago (2020-03-07 17:34:03 UTC) #2
hanwenn
On 2020/03/07 17:34:03, Dan Eble wrote: > The title of the review is "5824 Fix ...
4 months ago (2020-03-07 17:49:55 UTC) #3
hanwenn
make doc fix
4 months ago (2020-03-07 18:32:21 UTC) #4
Dan Eble
On 2020/03/07 17:49:55, hanwenn wrote: > it's a sequence of 6 commmits, which you can ...
4 months ago (2020-03-07 18:40:57 UTC) #5
Dan Eble
On 2020/03/07 17:49:55, hanwenn wrote: > it's a sequence of 6 commmits, which you can ...
4 months ago (2020-03-07 18:47:00 UTC) #6
hanwenn
On 2020/03/07 18:47:00, Dan Eble wrote: > On 2020/03/07 17:49:55, hanwenn wrote: > > it's ...
4 months ago (2020-03-07 18:53:21 UTC) #7
hanwenn
On 2020/03/07 18:40:57, Dan Eble wrote: > On 2020/03/07 17:49:55, hanwenn wrote: > > it's ...
4 months ago (2020-03-07 18:54:04 UTC) #8
hanwenn
On 2020/03/07 18:54:04, hanwenn wrote: > On 2020/03/07 18:40:57, Dan Eble wrote: > > On ...
4 months ago (2020-03-07 18:54:58 UTC) #9
hanwenn
On 2020/03/07 18:54:04, hanwenn wrote: > On 2020/03/07 18:40:57, Dan Eble wrote: > > On ...
4 months ago (2020-03-07 18:55:02 UTC) #10
Dan Eble
The logging and diffing changes basically LGTM (see my questions). I paid no attention to ...
4 months ago (2020-03-07 19:08:22 UTC) #11
Dan Eble
On 2020/03/07 18:55:02, hanwenn wrote: > > On 2020/03/07 18:40:57, Dan Eble wrote: > > ...
4 months ago (2020-03-07 19:16:04 UTC) #12
hanwenn
drop python/ fix
4 months ago (2020-03-07 19:21:31 UTC) #13
hanwenn
https://codereview.appspot.com/581770043/diff/571840081/scripts/build/output-distance.py File scripts/build/output-distance.py (left): https://codereview.appspot.com/581770043/diff/571840081/scripts/build/output-distance.py#oldcode1074 scripts/build/output-distance.py:1074: print('writing %s' % filename) On 2020/03/07 19:08:22, Dan Eble ...
4 months ago (2020-03-07 22:12:35 UTC) #14
hanwenn
backward compatibility; verify that make-check actually works
4 months ago (2020-03-08 09:46:43 UTC) #15
hahnjo
https://codereview.appspot.com/581770043/diff/563700043/scripts/build/GNUmakefile File scripts/build/GNUmakefile (right): https://codereview.appspot.com/581770043/diff/563700043/scripts/build/GNUmakefile#newcode19 scripts/build/GNUmakefile:19: $(PYTHON) output-distance.py --test This is wrong in 2 regards: ...
4 months ago (2020-03-11 10:16:52 UTC) #16
hanwenn
On Wed, Mar 11, 2020 at 11:16 AM <jonas.hahnfeld@gmail.com> wrote: > > > https://codereview.appspot.com/581770043/diff/563700043/scripts/build/GNUmakefile > ...
4 months ago (2020-03-11 11:49:17 UTC) #17
hanwenn
3 months, 3 weeks ago (2020-03-15 14:27:55 UTC) #18
On 2020/03/11 11:49:17, hanwenn wrote:
> On Wed, Mar 11, 2020 at 11:16 AM <mailto:jonas.hahnfeld@gmail.com> wrote:
> >
> >
> >
>
https://codereview.appspot.com/581770043/diff/563700043/scripts/build/GNUmake...
> > File scripts/build/GNUmakefile (right):
> >
> >
>
https://codereview.appspot.com/581770043/diff/563700043/scripts/build/GNUmake...
> > scripts/build/GNUmakefile:19: $(PYTHON) output-distance.py --test
> > This is wrong in 2 regards:
> > 1) This should be local-test as also discussed for python/
> 
> done.
> 
> > 2) output-distance.py is not copied over to the build tree, so my build
> > fails. I think patchy didn't see this problem because it's only running
> 
> done.
> 
> > 'make test' and James skipped 'make check' for the last patchset because
> > you wrote "This means you can't run a regression test between the state
> > before and after this change." (which I think is not true; at least it
> > worked for me).
> 
> I added backward compat code.
> 
>  http://codereview.appspot.com/563730043
> 
> > https://codereview.appspot.com/581770043/
> 
> 
> 
> -- 
> Han-Wen Nienhuys - mailto:hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

commit 5e87f62c45263940151f46e94b1cd2cee3f0c601
Merge: 3baa13d1a5 b02d93b9e8
Author: Han-Wen Nienhuys <hanwen@lilypond.org>
Date:   Wed Mar 11 09:00:59 2020 +0100

    Merge branch 'output-distance-test' into staging
    
    https://sourceforge.net/p/testlilyissues/issues/5824
    http://codereview.appspot.com/581770043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b