[webkit-reviews] review granted: [Bug 80932] Rebaselining for a new port doesn't work right with multiple fallback ports : [Attachment 131499] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 12 22:20:33 PDT 2012


Adam Barth <abarth at webkit.org> has granted Ojan Vafai <ojan at chromium.org>'s
request for review:
Bug 80932: Rebaselining for a new port doesn't work right with multiple
fallback ports
https://bugs.webkit.org/show_bug.cgi?id=80932

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=131499&action=review


> Tools/ChangeLog:10
> +	   Since chromium-leopard also falls back to mac-leopard, we need to
> +	   copy the existing result for both chromium-leopard and
chromium-snowleopard
> +	   before doing lion rebaselines.

I see.	Soooo complicated.  At some point we should decide that being parasitic
on apple-mac isn't worth the complexity.

> Tools/Scripts/webkitpy/tool/commands/rebaseline.py:71
> +    def _copy_existing_baseline(self,
platforms_to_move_existing_baselines_to, test_name, suffix):

This seems very likely to create non-optimized baselines.  Is there an
optimization step to clean up after these moves?

Moving baselines around is pretty complicated.	That's why, in the other class,
I went with the approach of preparing the whole move and then double-checking
that I didn't break anything (by seeing that the before/after results were the
same).	I wonder if an approach that would be useful here...

One last comment is that this command has grown out of control.  It really
needs to be refactored into smaller interacting pieces rather than having all
the logic in one giant class.  (That's perhaps a yak for another day, however.)


More information about the webkit-reviews mailing list