[Webkit-unassigned] [Bug 38918] add a FancyReviewQueue to upload r? patches to rietveld

[bugzilla-daemon at webkit.org]

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





--- Comment #23 from Ojan Vafai <ojan at chromium.org>  2010-05-24 12:05:45 PST ---
(In reply to comment #20)
> WebKitTools/Scripts/webkitpy/common/net/bugzilla.py:629
>  +          self.browser['flag_type-4'] = ('?',)
> It's too bad that this will only work for patches uploaded with webkit-patch.  What's the plan for getting all r? patches uploaded?  Why not start with that?

I was just trying to scope the change to avoid creating havoc if something goes wrong with the queue. But, whatever, the queue now processes r? and in-rietveld? matches.

> WebKitTools/Scripts/webkitpy/common/net/bugzilla.py:805
>  +              self.browser.set_value(comment_text, name='comment', nr=0)
> I don't understand how this change relates...

I had made the change in an earlier iteration of this patch and it seemed like a good one. Anyways, removed.

> WebKitTools/Scripts/webkitpy/tool/commands/queues.py:336
>  +          comment_text = "Rejecting patch %s from rietveld-queue." % patch.id()
> Why might this happen?  Do we want to send email when it happens?

Only case I can think of is if rietveld is down. This will post to the bug, so no need to send a separate email, right?

> WebKitTools/Scripts/webkitpy/tool/commands/queues.py:345
>  +      def _error_message_for_bug(tool, status_id, script_error):
> This function looks like a copy/paste of another function.  Can we share the implementation?

Moved into StepSequenceErrorHandler. Not sure if that is the right place. I could create a AbstractPatchQueue/StepSequenceErrorHandler superclass, but that seems excessive. Putting in directly in AbstractPatchQueue seemed wrong since this used in an implementation of a StepSequenceErrorHandler method.

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