[webkit-reviews] review granted: [Bug 31904] [bzt] Kill WebKitLandingScripts : [Attachment 43906] Step 6

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 31904: [bzt] Kill WebKitLandingScripts
https://bugs.webkit.org/show_bug.cgi?id=31904

Attachment 43906: Step 6
https://bugs.webkit.org/attachment.cgi?id=43906&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
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.