[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 #25 from Ojan Vafai <ojan at chromium.org>  2010-06-03 13:49:01 PST ---
(In reply to comment #24)
> (From update of attachment 56910 [details])
> 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.

The original code was just dumb. It's stripping the first argument. But upload.py already does that. So we were doing it twice.

> 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.

Tried to make the text more meaningful.

> 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.

Added a FIXME for now. I was going to just do this, but I got stuck on testing it.

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