[webkit-reviews] review denied: [Bug 90504] Improve webkit-patch rebaseline to work for more cases : [Attachment 150684] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jul 4 07:43:53 PDT 2012
Adam Barth <abarth at webkit.org> has denied review:
Bug 90504: Improve webkit-patch rebaseline to work for more cases
https://bugs.webkit.org/show_bug.cgi?id=90504
Attachment 150684: Patch
https://bugs.webkit.org/attachment.cgi?id=150684&action=review
------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=150684&action=review
> Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py:243
> + results_directory = "results/layout-test-results" if
self._builder.name().startswith('Webkit') else "r%s (%s)" % (self.revision(),
self._number)
The more I think about this line, the more it bothers me. The fact that the
Chromium builders have names that misspell WebKit is something I'd like to fix
some day. Relying upon that error here is really crappy.
This line also contains the string "results/layout-test-results", which repeats
a constant in accumulated_results_url(). Rather than repeating ourselves, we
should call accumulated_results_url(). In fact, that's key to avoiding
introspecting the builder's name. Rather than looking for the "Webkit" typo,
we should check whether accumulated_results_url() is truthy.
Actually, now that I think about this further, this change is just plain wrong.
results_url() no longer returns the results URL for this Build on Chromium
buildbots. Instead, it returns the accumulated_results_url, which contains
results other than the ones for this build. Rather than make this function
wrong, we need to make the callers of this function smarter.
More information about the webkit-reviews
mailing list