[webkit-reviews] review granted: [Bug 38918] add a FancyReviewQueue to upload r? patches to rietveld : [Attachment 56910] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat May 29 09:40:17 PDT 2010


Adam Barth <abarth at webkit.org> has granted Ojan Vafai <ojan at chromium.org>'s
request for review:
Bug 38918: add a FancyReviewQueue to upload r? patches to rietveld
https://bugs.webkit.org/show_bug.cgi?id=38918

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
Minor nits below.  You should feel free to either address them before landing
or in a follow-up patch.

WebKitTools/Scripts/webkitpy/common/net/rietveld.py:77
 +	    issue, patchset = upload.RealMain(args, data=diff)
I don't quite understand this change, but ok.

WebKitTools/Scripts/webkitpy/tool/commands/queues.py:329
 +	    comment_text = "Rejecting patch %s from rietveld-queue." %
patch.id()
I still worry that this message is cryptic and folks won't understand it.  We
can iterate after landing though.

WebKitTools/Scripts/webkitpy/tool/commands/stepsequence.py:39
 +	def _error_message_for_bug(tool, status_id, script_error):
This isn't quite the right place for this method.  StepSequenceErrorHandler is
just an interface definition.  It shouldn't have any implementations.  We need
to create an intermediate class that inherits from StepSequenceErrorHandler
that the queues inherit from.


More information about the webkit-reviews mailing list