Summary: | webkit-patch: Passing --no-review should submit patch to EWS by default | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||||
Component: | Tools / Tests | Assignee: | Daniel Bates <dbates> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ap, buildbot, darin, dbates, ddkilzer, glenn, joepeck, sam, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=54735 | ||||||||
Attachments: |
|
Description
BJ Burg
2015-09-05 09:39:10 PDT
An alternative approach proposed in bug #54735 is to introduce a new "try" command "that basically [would] just be upload with --no-review and a pre-set title." I am leaning towards implementing this as proposed in this bug (comment #0) - as an optional flag - because then it could be supported by all of the applicable webkit-patch commands that post a patch: upload, post, and post-commits. The proposal in comment #0 suggests that the flag, --request-ews, just does one thing: submit the patch to EWS. Alternatively we could have the flag, maybe called --try, do more than one things, say do not mark the patch for review and submit the patch to EWS. Or even, upload the patch with a hardcoded name "For EWS Bots" (suggested by Eric Seidel in bug #54735, comment 1), do not mark the patch for review, and submit the patch to EWS. Thoughts? Created attachment 317914 [details] Patch and unit tests Here is a patch that adds a --submit-to-ews flag to commands upload, post, and post-commits. Its usage matches the proposal in comment #0: "webkit-patch upload --no-review --submit-to-ews ...". That is, it must be used in conjunction with --no-review to be effective. I also added a new command, submit-to-ews, that can take one or more attachment ids and submit them to EWS. This command can be used to submit an existing --no-review patch to EWS. Looks good. I would use this. I wish "--submit-to-ews" was something shorter. Also annoying since I will always have to do both "--no-review" and "--submit-to-ews". Maybe submitting to EWS should be the default even when you say "--no-review" and there should be an option "--no-ews". Created attachment 318045 [details]
Patch and unit tests
(In reply to Darin Adler from comment #3) > Maybe submitting to EWS should be the default even when you say "--no-review" and there should be an option "--no-ews". I updated the patch to take this approach. Comment on attachment 318045 [details]
Patch and unit tests
r=me
Actually, I am not going to land the dedicated submit-to-ews command part of the patch. I am not going to include this command because I do not see much value to adding it at least not in its current form that requires specifying attachment id(s), which can only be discovered by visiting the bug. If a person visits the bug then it is straightforward to click the Submit for EWS on the attachments they want to submit. We can always add such a submit-to-ews command in a follow up if more realistic use cases materialize. Committed r220715: <http://trac.webkit.org/changeset/220715> |