| Summary: | [bzt] Kill WebKitLandingScripts | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||||||||||||
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | eric | ||||||||||||||
| Priority: | P2 | ||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||
| Hardware: | All | ||||||||||||||||
| OS: | All | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Adam Barth
2009-11-25 23:51:23 PST
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> |