Bug 148899 – webkit-patch: Passing --no-review should submit patch to EWS by default

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Blaze Burg]

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.

[Daniel Bates]

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?

[Daniel Bates]

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.

[Darin Adler]

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".

[Daniel Bates]

Created attachment 318045 [details]
Patch and unit tests

[Daniel Bates]

(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.

[David Kilzer (:ddkilzer)]

Comment on attachment 318045 [details]
Patch and unit tests

r=me

[Daniel Bates]

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.

[Daniel Bates]

Committed r220715: <http://trac.webkit.org/changeset/220715>

[Radar WebKit Bug Importer]

<rdar://problem/33883377>