lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: output-distance: treat non-existent files as empty string (issue 583


From: nine . fierce . ballads
Subject: Re: output-distance: treat non-existent files as empty string (issue 583590043 by address@hidden)
Date: Sat, 29 Feb 2020 07:51:43 -0800

https://codereview.appspot.com/583590043/diff/571790043/scripts/build/output-distance.py
File scripts/build/output-distance.py (right):

https://codereview.appspot.com/583590043/diff/571790043/scripts/build/output-distance.py#newcode531
scripts/build/output-distance.py:531: if self.contents[0] is None !=
self.contents[1] is None:
This symmetric condition no longer matches the comment immediately
below.  It seems that you would also avoid showing the diff when the
baseline version has a file that is missing in the new version.  I have
mixed opinions about that.

In the case of an intentional removal of a test case, I probably
wouldn't mind seeing a striped cell on the right-hand side of the page,
but I think that might be inconsistent with the way other types of files
are still being handled in this case.  IIRC, removed test cases
currently do not appear in the results except as a count, and I slightly
prefer keeping it that way.

In the case of an error causing an expected text file to be missing, I
think I would prefer output that is harder to overlook.  The full diff
output will probably qualify as harder to overlook, though it's not the
only option.  Having the script fail on None.strip(), though not the
best option, is still better than treating this as discreetly as the
intentional removal of a test case.

My first approach would probably have been to leave this earlier stuff
alone and address the issue below (untested):

-   cont1 = self.contents[1].strip();
+   cont1 = self.contents[1].strip() if self.contents[1] else ''

These changes deserve some systematic manual testing; I hope you're
covering that.  If this script fails to publish differences when it
should, nobody else involved in the countdown process is going to
notice.

https://codereview.appspot.com/583590043/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]