[Webkit-unassigned] [Bug 50098] New webkit-patch rebaseline2 command.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=50098





--- Comment #11 from Dirk Pranke <dpranke at chromium.org>  2010-12-01 14:38:07 PST ---
(From update of attachment 75263)
I just started looking at this patch (sorry for the delay!), so I will confine myself to a couple of comments on topics others haven't raised, and then reply to some of the other bug comments.

Mihai says that the intent of this is to replace both webkit-patch rebaseline and rebaseline-chromium-webkit-tests. If so, great! The latter code is poorly tested and hard to maintain, so it'll be nice to have something theoretically better :)


> WebKitTools/Scripts/webkitpy/common/system/filesystem.py:126
> +        shutil.copyfile(src, dest)

Why copy_file instead of copyfile(), or remove_file() instead of unlink or remove? I would prefer to have the methods be the same as the underlying package/class method names, to minimize the cognitive overhead.

> WebKitTools/Scripts/webkitpy/layout_tests/port/base.py:637
> +

Comment: I'm not wild about this being put into the layout_test/port/* classes, mostly because this has nothing to do with *running* the tests, and partially because this class is already large and hard to understand as a whole. That said, there's not an obvious other place to put them, and I don't want to create a parallel port tree, and it's only one entry point. This is probably something to think about when I/we get around to refactoring/combining this code with the common.config.ports code.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.