[Webkit-unassigned] [Bug 51176] sheriff-bot should be able to do multi-revision rollouts

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Gabor Rapcsanyi <rgabor at inf.u-szeged.hu>  2011-01-04 02:20:41 PST ---
(In reply to comment #2)
> (From update of attachment 76757 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=76757&action=review
> 
> Needs another round. :)
> 
> > WebKitTools/Scripts/webkitpy/tool/bot/irc_command.py:64
> > +        read_revision = True
> > +        rollout_reason = []
> > +        # the first argument must be a revision number
> > +        svn_revision_list = [args[0].lstrip("r")]
> > +        if not svn_revision_list[0].isdigit():
> > +            read_revision = False
> > +
> 
> Please split this (arg parsing, etc.) into private (_foo()) helper functions instead of just making execute longer.
> 

corrected

> > WebKitTools/Scripts/webkitpy/tool/bot/sheriff.py:58
> > +            svn_revisions = " ".join([str(int(revision)) for revision in svn_revision_list])
> 
> Um, why do str(int(?
> 

Here I just check the revisions whether they are numbers and after I convert back to string for the concatenation.

> > WebKitTools/Scripts/webkitpy/tool/bot/sheriff.py:76
> > +            svn_revisions,
> 
> Please pass them individually as numbers converted to strings, instead of as a joined string.

We have to pass the revisions between apostrophes or quotes to webkit-patch, and I think that's the best place to convert them to that format.

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