Bug 240945

Summary:	`git-webkit pr` requires uncommitted changes to do any work
Product:	WebKit	Reporter:	Kenneth Russell <kbr>
Component:	Tools / Tests	Assignee:	Nobody <webkit-unassigned>
Status:	RESOLVED WONTFIX
Severity:	Major	CC:	ap, jbedard, kkinnunen, webkit-bug-importer
Priority:	P2	Keywords:	InRadar
Version:	WebKit Nightly Build
Hardware:	Unspecified
OS:	Unspecified
Bug Depends on:	241238
Bug Blocks:	239082

Description Kenneth Russell 2022-05-25 23:22:21 PDT

`git-webkit pr` requires uncommitted changes in order to do any work. Otherwise, it reports:

No modified files

This single design decision breaks git workflows badly, and is a regression compared to "webkit-patch upload". In the past couple of days using the script, I've already forgotten multiple times to not commit my changes locally, both to my own branches when initiating a pull request, and to existing pull request branches under the eng/ namespace.

Please make the following improvements:

1) If on a user's local branch (not one which "git-webkit pr" created), and all changes have already been committed, create a new branch under the eng/ namespace and follow the current workflow to incorporate all commits relative to origin/main (or to the upstream branch - however the script currently works), produce the commit message from a template, and upload it.

2) If on one of the eng/ branches, and all changes have been committed, allow "git-webkit pr --amend" or just "git-webkit pr" to push the current set of commits to the already-open pull request.

Thanks in advance for implementing this.

Comment 1 Alexey Proskuryakov 2022-05-30 19:34:18 PDT

Could you please post your exact steps to reproduce? `git webkit pr` is most definitely supposed to work with committed changes on branches, and while I personally haven't tried that yet, I'm pretty sure that this is how most people use it.

Comment 2 Kenneth Russell 2022-06-01 16:16:15 PDT

Previously, my steps were:

1) Create a PR via `git-webkit pr`

2) Add another commit on that branch (previously it was eng/branch-name)

3) Run `git-webkit pr` again

Now however I'm not sure these steps reproduce - will have to try creating a test branch. Since I tried this, the creation of the eng/ branches has been removed from `git-webkit pr`.

Comment 3 Alexey Proskuryakov 2022-06-01 16:59:09 PDT

Is your expectation that step 3 creates a new branch? I would expect it to push the new commit to the same PR that got created in step 1. And I believe that this is how it works today.

Comment 4 Kenneth Russell 2022-06-01 17:04:40 PDT

I'd also expect it to push a new commit to the same PR from step 1. When I filed this bug, `git-webkit pr` instead reported "No modified files" and bailed out without pushing any new commits to that PR.

Comment 5 Radar WebKit Bug Importer 2022-06-01 23:23:12 PDT

<rdar://problem/94269387>

Comment 6 Kenneth Russell 2022-06-02 15:47:00 PDT

At this point, `git-webkit pr --amend` will upload the same branch to an existing PR even if all changes have been committed.

One caveat is that if multiple commits are made on the branch, all of their commit messages seem to show up concatenated in the PR. For this reason, it seems the expected workflow is to use `git commit --amend` locally. Personally, I would prefer if commits were auto-squashed upon PR upload or amending, because this makes it easier to keep local branch history especially with long-running branches, but this is a matter of opinion.

Please do keep the documentation for git-webkit up to date; https://github.com/WebKit/WebKit/wiki/Pull-Requests is out of date now (it doesn't use eng/-prefixed branches any more, for example). Thanks.