Bug 148899 - webkit-patch: Passing --no-review should submit patch to EWS by default
Summary: webkit-patch: Passing --no-review should submit patch to EWS by default
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-09-05 09:39 PDT by BJ Burg
Modified: 2017-08-14 14:15 PDT (History)
9 users (show)

See Also:


Attachments
Patch and unit tests (17.93 KB, patch)
2017-08-10 21:18 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch and unit tests (18.18 KB, patch)
2017-08-14 10:00 PDT, Daniel Bates
ddkilzer: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2015-09-05 09:39:10 PDT
If you are a Good Person and use EWS to avoid build breaks, it's really tedious to set r? on a patch that's already reviewed then get a bunch of bug mail for no reason. It's also tedious to click "Submit For EWS" on every patch. There should just be an option, like

$ webkit-patch upload --request-ews --no-review

Which will click that button for us.
Comment 1 Daniel Bates 2017-08-10 20:59:49 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?
Comment 2 Daniel Bates 2017-08-10 21:18:53 PDT
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.
Comment 3 Darin Adler 2017-08-14 09:12:24 PDT
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".
Comment 4 Daniel Bates 2017-08-14 10:00:45 PDT
Created attachment 318045 [details]
Patch and unit tests
Comment 5 Daniel Bates 2017-08-14 10:01:45 PDT
(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 6 David Kilzer (:ddkilzer) 2017-08-14 10:26:20 PDT
Comment on attachment 318045 [details]
Patch and unit tests

r=me
Comment 7 Daniel Bates 2017-08-14 14:08:56 PDT
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.
Comment 8 Daniel Bates 2017-08-14 14:14:19 PDT
Committed r220715: <http://trac.webkit.org/changeset/220715>
Comment 9 Radar WebKit Bug Importer 2017-08-14 14:15:36 PDT
<rdar://problem/33883377>