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>