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 | ||
Kenneth Russell
`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.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Alexey Proskuryakov
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.
Kenneth Russell
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`.
Alexey Proskuryakov
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.
Kenneth Russell
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.
Radar WebKit Bug Importer
<rdar://problem/94269387>
Kenneth Russell
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.