Bug 208775

Created attachment 392903 [details]
Patch

Can you also rename rollout to revert in https://trac.webkit.org/browser/webkit/trunk/Tools/BuildSlaveSupport/ews-build/steps.py and steps_unittest.py

(In reply to Aakash Jain from comment #2)
> Can you also rename rollout to revert in
> https://trac.webkit.org/browser/webkit/trunk/Tools/BuildSlaveSupport/ews-
> build/steps.py and steps_unittest.py

Oh shoot, my local pull is out of date and I didn't have those additions. Thanks for pointing that out!

Created attachment 392910 [details]
Patch

Comment on attachment 392910 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=392910&action=review

> Tools/ChangeLog:3
> +        [Tools] Out with rollouts, in with reverts

This bug title was too confusing.

> Tools/Scripts/webkitpy/common/net/bugzilla/attachment.py:84
> +    def is_revert(self):
> +        return self.name().startswith(self.revert_preamble)

We need to check both rollout & revert here
because this function is used to check whether something is a rollout or a revert.

> Tools/Scripts/webkitpy/tool/bot/irc_command.py:156
> +    usage_string = "revert SVN_REVISION [SVN_REVISIONS] REASON"
> +    help_string = "Opens a revert bug, CCing author + reviewer, and attaching the reverse-diff of the given revisions marked as commit-queue=?."

While we're at, we should rephrase this as "Opens a bug to revert the specified revision, ..."

> Tools/Scripts/webkitpy/tool/bot/irc_command.py:245
> -            tool.irc().post("%s: Created rollout: %s" % (nicks_string, bug_url))
> +            tool.irc().post("%s: Created revert: %s" % (nicks_string, bug_url))

While we're at it, we should rephrase it to "Created a revert patch":

> Tools/Scripts/webkitpy/tool/commands/newcommitbot.py:157
> -                return '%s rolled out %s in %s : %s' % (author, rollout.group('revisions'),
> +                return '%s rolled out %s in %s : %s' % (author, revert.group('revisions'),

We should rephrase this as "reverted" as well.

> Tools/Scripts/webkitpy/tool/commands/newcommitbot.py:159
> -            lines[0] = '%s rolled out %s in %s' % (author, rollout.group('revisions'), linkified_revision)
> +            lines[0] = '%s rolled out %s in %s' % (author, revert.group('revisions'), linkified_revision)

Ditto.

Created attachment 392917 [details]
Patch for landing

Thanks for the review!

(In reply to Ryosuke Niwa from comment #5)
> > Tools/ChangeLog:3
> > +        [Tools] Out with rollouts, in with reverts
> 
> This bug title was too confusing.

Haha, I may have been having a bit too much fun there. :P

Services EWS seems unwilling to notice changes to its unit tests -- Aakash, can you confirm whether this failure looks legitimate to you?

Created attachment 392929 [details]
Patch for landing

(In reply to Ross Kirsling from comment #8)
> Services EWS seems unwilling to notice changes to its unit tests -- Aakash, can you confirm whether this failure looks legitimate to you?
The patch seems to be have missed replacing one instance of rollout in steps_unittest.py

Created attachment 392933 [details]
Patch for landing

(In reply to Aakash Jain from comment #10)
> (In reply to Ross Kirsling from comment #8)
> > Services EWS seems unwilling to notice changes to its unit tests -- Aakash, can you confirm whether this failure looks legitimate to you?
> The patch seems to be have missed replacing one instance of rollout in
> steps_unittest.py

I didn't realize there were multiple changes for this landing simultaneously -- rebased again.

Comment on attachment 392933 [details]
Patch for landing

Clearing flags on attachment: 392933

Committed r258097: <https://trac.webkit.org/changeset/258097>

All reviewed patches have been landed.  Closing bug.

<rdar://problem/60195742>