[Webkit-unassigned] [Bug 52136] Add TestOutput classes to webkitpy.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 3 20:55:07 PST 2011


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





--- Comment #20 from James Kozianski <koz at chromium.org>  2011-02-03 20:55:07 PST ---
(In reply to comment #17)
> (From update of attachment 79891 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=79891&action=review
> 
> >>> Tools/Scripts/webkitpy/common/net/testoutput.py:105
> >>> +    def is_newer_than(self, other):
> >> 
> >> ?
> > 
> > This is useful for rebaselining - if one TestOutput is 'newer' than the other, then it means that we can overwrite the old one.
> 
> As we said in person is_rebaseline_of is more clear and serves the same purpose.

Done.

> 
> > Tools/Scripts/webkitpy/common/net/testoutput.py:111
> > +    def is_rebaseline_of(self, other):
> 
> s/is_rebaseline_of/is_new_baseline_for/ ?

Done.

> 
> >>> Tools/Scripts/webkitpy/common/net/testoutput.py:128
> >>> +    def _path_to_platform(self, platform):
> >> 
> >> Why pass in platform instead of just using self._platform?
> > 
> > A previous comment from eseidel: "Seems like the first part of this is some sort of install_path function instead?  I could see other pieces of code wanting to have some way to generate paths from platform, file pairs, no?"
> > 
> > It seems like overkill to put it such a function in its own module though (and where exactly such a module would be put is another question).
> 
> But this function doesn't take platform/file pairs, it just takes the platform. I see what Eric's original comment was getting at, but I don't think it actually makes sense. It would if we were ultimately creating just a single string. But we're creating a path and a filename. In either case, I think having this as a separate function is easier to read, but it's not useful enough to try to make generic. Certainly as is, it doesn't make sense to me to pass in platform as an argument.
> 
> It's not a big deal either way though.

Yep. Done.

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



More information about the webkit-unassigned mailing list