As it stands, there are many special cases and mystifying helper objects. The special cases should be organized more uniformly, and the mystifying helper objects should be removed.
Created attachment 240236 [details] Refactoring!
Comment on attachment 240236 [details] Refactoring! View in context: https://bugs.webkit.org/attachment.cgi?id=240236&action=review > Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py:191 > + if (len(results.failing_tests()) - len(self._clean_tree_results.failing_tests())) <= 5: > + return False I think that this code would benefit from somewhat more comments. E.g., here we could explain that the clean tree has so many failures that we can't be certain if having a few more means that the patch introduces a regression. > Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py:245 > - return False > + return True I became confused here. Perhaps the best path forward is to walk over the patch line by line in person - or maybe you can get a review from someone who really knows Python and can read it more easily.
Created attachment 240285 [details] Adds some comments, and fixes a bug
Comment on attachment 240285 [details] Adds some comments, and fixes a bug Clearing flags on attachment: 240285 Committed r175063: <http://trac.webkit.org/changeset/175063>
All reviewed patches have been landed. Closing bug.
Created attachment 240298 [details] Adds FIXMEs to tests, and sets the test expectations to agree with current behavior.
Comment on attachment 240298 [details] Adds FIXMEs to tests, and sets the test expectations to agree with current behavior. Commented on the wrong bug..