[webkit-reviews] review denied: [Bug 66837] Parse reftest.list and extract types of ref tests : [Attachment 115719] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 66837: Parse reftest.list and extract types of ref tests
https://bugs.webkit.org/show_bug.cgi?id=66837

Attachment 115719: Patch
https://bugs.webkit.org/attachment.cgi?id=115719&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=115719&action=review


> Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:342
> +    def _parse_all_reftest_lists(self):

Why wouldn't this move onto a separate RefTestListParser object?  (Or something
better named).	Why should the manager have this functionality?

> Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:366
> +    def _parse_reftest_list(self, reftest_list, test_dir):

This name doesnt' convey the fact that it's going to store the results in some
variable on self.  I would expect it to either return something, or be named
differently.