Bug 223942

Summary: Add confirmation prompt to discourage manual commits
Product: WebKit Reporter: Aakash Jain <aakash_jain>
Component: Tools / TestsAssignee: Aakash Jain <aakash_jain>
Status: ASSIGNED ---    
Severity: Normal CC: aakash_jain, ap, ews-watchlist, glenn, jbedard, ryanhaddad, 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=223954
Attachments:
Description Flags
Patch aakash_jain: review?, ews-feeder: commit-queue-

Description Aakash Jain 2021-03-30 11:37:37 PDT
Add a confirmation prompt which appears while doing manual committing (e.g.: through webkit-patch land) to discourage manual commits. Manual commits will not be allowed after we move to github. So, it would be good to provide such warning so that people can either start using commit-queue, or let us know the reason for manual commits, so that commit-queue can be improved.
Comment 1 Aakash Jain 2021-03-30 11:58:27 PDT
Created attachment 424672 [details]
Patch
Comment 2 Aakash Jain 2021-03-30 12:05:26 PDT
unit-tests needs to be updated, but would be nice to have it reviewed meanwhile.
Comment 3 Jonathan Bedard 2021-03-30 12:18:07 PDT
Comment on attachment 424672 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=424672&action=review

Looks good, just need to update the unit tests.

> Tools/Scripts/webkitpy/tool/steps/commit.py:72
> +        warning_message += '\nIn future, when WebKit moves to github, manual commiting would not be allowed.'

github -> GitHub
Comment 4 Sam Weinig 2021-03-30 12:22:31 PDT
Comment on attachment 424672 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=424672&action=review

> Tools/Scripts/webkitpy/tool/steps/commit.py:74
> +        warning_message += '\nIn you have a specific reason for manual commit, please let us know at admin@webkit.org or please file a bug.'
> +        warning_message += '\nAre you sure you want to continue with manual committing?'

Manual committing is very useful for quick typo fixes and build fixes. This doesn't seem like a great new rule to push on the community, especially if we don't have big problem with it.
Comment 5 Jonathan Bedard 2021-03-30 12:38:44 PDT
(In reply to Sam Weinig from comment #4)
> Comment on attachment 424672 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=424672&action=review
> 
> > Tools/Scripts/webkitpy/tool/steps/commit.py:74
> > +        warning_message += '\nIn you have a specific reason for manual commit, please let us know at admin@webkit.org or please file a bug.'
> > +        warning_message += '\nAre you sure you want to continue with manual committing?'
> 
> Manual committing is very useful for quick typo fixes and build fixes. This
> doesn't seem like a great new rule to push on the community, especially if
> we don't have big problem with it.

We don't have a great way to do any pre-commit validation in GitHub the way we do in Subversion. This is important for things like enforcing rules with tabs, adding identifiers to commit messages, rejecting changes with "OOPS" in them.

To address build-fix/type style changes, the plan is to have a fast-path commit-queue that works the way current commit queue does when EWS was successful.
Comment 6 Aakash Jain 2021-04-01 13:55:03 PDT
> To address build-fix/type style changes, the plan is to have a fast-path commit-queue that works the way current commit queue does when EWS was successful.
Note that the fast-commit-queue will also skip Building. It should take it only ~1 minute to land the patch. We just add the fast-commit-queue mode in https://bugs.webkit.org/show_bug.cgi?id=223954. We also plan to add webkit-patch support for it (e.g.: using --fast-cq parameter).
Comment 7 Radar WebKit Bug Importer 2021-04-06 11:38:17 PDT
<rdar://problem/76276805>