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: On 2020/02/29 15:51:43, Dan Eble wrote: > 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. I adjusted the comment. > 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. I disagree. Having the script crash prevents us from forming a complete impression of what a change does, because the crash can hide other regression failures. > 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 '' this change is to address the failures occurring in https://sourceforge.net/p/testlilyissues/issues/5785/ caused by contents[0] being None. > 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. this is why I am extending the if case to be symmetric. We already know that the addition of files is handled correctly. Making a minimal change here avoids some QA. https://codereview.appspot.com/583590043/