It is an evil static of doom that prevents us from testing download commands.
Created attachment 43901 [details] step 1
Created attachment 43902 [details] step 2
Created attachment 43903 [details] step 3
Created attachment 43904 [details] Step 4
Created attachment 43905 [details] Step 5
Created attachment 43906 [details] Step 6
Comment on attachment 43901 [details] step 1 Why pass tool? 119 def setup_for_patch_apply(tool, options): Seems scm is cleaner there. This looks good though.
Comment on attachment 43902 [details] step 2 Calling error() is evil and wrong. We need to fix that at some point.
Comment on attachment 43903 [details] step 3 LGTM.
Comment on attachment 43904 [details] Step 4 Commands already have a self.tool now. No need to pass tool: self._create_changelogs_for_revert(tool.scm(), revision) 398 self._create_changelogs_for_revert(tool, revision) If you're going to move away from scm(), we should move to self.tool
Comment on attachment 43905 [details] Step 5 LGTM. Eventually we need to unify SCM.run_command with this too.
Comment on attachment 43906 [details] Step 6 I intentionally had this take an scm param: commit_message = commit_message_for_this_commit(scm) 135 commit_message = scm.commit_message_for_this_commit() It doesn't belong on SCM. Our commit message junk doesn't belong in scm.py. 38 # FIXME: The options should really live on each "Step" object. I LOVE the idea of a Step object, and even more that options would be automatically assembled from the set of step objects! rollout used to work just fine: comment_text = WebKitLandingScripts.build_and_commit(tool.scm(), options) 407 # FIXME: This function does not exist!! 408 # comment_text = WebKitLandingScripts.build_and_commit(tool.scm(), options) 409 raise ScriptError("OOPS! This option is not implemented (yet).") Not sure when it broke. 406410 self._reopen_bug_after_rollout(tool, bug_id, comment_text) is broken in the current bugzilla however, because there is no longer a "REOPENED" state in the popup. I guess we could add a big fixme next to commit_message_for_this_commit. It doesn't belong in scm.py. :( I'd like you to at least add a FIXME.
Comment on attachment 43901 [details] step 1 Clearing flags on attachment: 43901 Committed r51431: <http://trac.webkit.org/changeset/51431>
Comment on attachment 43902 [details] step 2 Clearing flags on attachment: 43902 Committed r51432: <http://trac.webkit.org/changeset/51432>
Comment on attachment 43903 [details] step 3 Clearing flags on attachment: 43903 Committed r51433: <http://trac.webkit.org/changeset/51433>
Comment on attachment 43904 [details] Step 4 Clearing flags on attachment: 43904 Committed r51434: <http://trac.webkit.org/changeset/51434>
Comment on attachment 43905 [details] Step 5 Clearing flags on attachment: 43905 Committed r51435: <http://trac.webkit.org/changeset/51435>
Committed r51437: <http://trac.webkit.org/changeset/51437>