125

I opened a pull request to rails repo on github by using Fork & Edit this file file button.

Now, After getting feedback on my PR, I wanted to add some more commits. so here is what I ended by doing

$ git clone git@github.com:gaurish/rails.git #my forked repo
$ git rebase -i 785a2e5 #commit hash of my commit using which PR was opened
$ git checkout patch-3 #branch name I had to send my commits under to be shown in that PR
$ git commit -am "Changes done as per feedback"
$ git push origin patch-3

This worked fine but seems quite a complex workflow. Maybe I am wrong something wrong here?

my question is: Am I doing this the correct way? if not, then what is the proper way to do this?

hippietrail
  • 15,848
  • 18
  • 99
  • 158
CuriousMind
  • 33,537
  • 28
  • 98
  • 137
  • 4
    Some coming here may find this fits their scenario better: http://stackoverflow.com/questions/9790448/how-to-update-a-pull-request – AaronLS Aug 04 '13 at 07:08
  • 4
    I also found this version of the question/answer clearer: http://stackoverflow.com/questions/7947322/preferred-github-workflow-for-updating-a-pull-request-after-code-review?rq=1 – Ben Wheeler Jan 06 '14 at 15:59

6 Answers6

85

Since you're using GitHub's tools and just changing one file, you could also browse to the file on GitHub, select the proper branch from the upper left corner under the "tree:" dropdown (patch-3 in your case), and now choose "Edit this file". Now your changes will be committed to this branch and will show up in your pull request

MrE
  • 19,584
  • 12
  • 87
  • 105
Abe Voelker
  • 30,124
  • 14
  • 81
  • 98
  • Note this won't work if the branch is [protected](https://stackoverflow.com/questions/55299930/editing-files-as-a-collaborator-on-github#55309409), the edit button will be greyed out – Louis Maddox Dec 21 '20 at 18:17
  • You also must be on the repo the PR is originating from, e.g. if you're merging into an upstream repo, you can only do the above if you go to your fork. – stason Jan 21 '22 at 21:05
60

Yeah - you are doing a lot more work than you need to. Just make an additional commit and then force push it. You'll see the original commit as well as then newly pushed one when you refresh github in your browser.

$ git commit -m "These changes are in response to PR comments"
$ git push -f origin HEAD
Forrest
  • 2,968
  • 1
  • 27
  • 18
11

I just recently blogged on this topic:

How do we keep this feature branch up to date? Merging the newest upstream commits is easy, but you want to avoid creating a merge commit, as that won’t be appreciated when pushed to upstream: you are then effectively re-committing upstream changes, and those upstream commits will get a new hash (as they get a new parent). This is especially important, as those merged commits would be reflected in your GitHub pull request when you push those updates to your personal GitHub feature branch (even if you do that after you issued the pull request.)

That’s why we need to rebase instead of merging:

git co devel #devel is ansible's HEAD aka "master" branch
git pull --rebase upstream devel
git co user-non-unique
git rebase devel

Both the rebase option and rebase command to git will keep your tree clean, and avoid having merge commits. But keep in mind that those are your first commits (with which you issued your first pull request) that are being rebased, and which now have a new commit hash, which is different from the original hashes that are still in your remote github repo branch.

Now, pushing those updates out to your personal GitHub feature branch will fail here, as both branches differ: the local branch tree and the remote branch tree are “out of sync”, because of those different commit hashes. Git will tell you to first git pull --rebase, then push again, but this won’t be a simple fast-forward push, as your history got rewritten. Don’t do that!

The problem here is that you would again fetch your first changed commits as they were originally, and those will get merged on top of your local branch. Because of the out-of-sync state, this pull does not apply cleanly. You’ll get a broken history where your commits appear two times. When you push all of this to your GitHub feature branch, those changes will get reflected on the original pull request, which will get very, very ugly.

AFAIK, there is actually no totally clean solution to this. The best solution I found is to force push your local branch to your GitHub branch (actually forcing a non-fast-forward update):

As per git-push(1):

Update the origin repository’s remote branch with local branch, allowing non-fast-forward updates. This can leave unreferenced commits dangling in the origin repository.

So don’t pull, just force push like this:

git push svg +user-non-unique

or:

git push svg user-non-unique --force

This will actually plainly overwrite your remote branch, with everything in your local branch. The commits which are in the remote stream (and caused the failure) will remain there, but will be dangling commits, which will eventually get deleted by git-gc(1). No big deal.

As I said, this is AFAICS the cleanest solution. The downside of this, is that your PR will be updated with those newest commits, which will get a later date, and could appear out of sync in the comment history of the PR. No big problem, but it could potentially be confusing.

aschultz
  • 1,658
  • 3
  • 20
  • 30
7

You could also create a new pull request which is bound to master instead of a specific abc1234 revision.

That way, any new commit/push to your repository will be added to the pull request.

cfedermann
  • 3,286
  • 19
  • 24
2

I was able to add more commits to an existing pull request by following these steps:

  1. Push the newest commits to your feature branch
  2. In GitHub, go to the feature branch and click 'New Pull Request'
  3. You should be able to see the previously open pull request in the following screen. Now click on 'View Pull Request': enter image description here
  4. Your newest commits should be added to your existing pull request now. Leave a comment if necessary
flyingfishcattle
  • 1,817
  • 3
  • 14
  • 25
  • 2
    Doesn't work. You indeed see the previous MR, but no more. The new commits aren't added 'magically'. – orzel Oct 11 '22 at 22:20
0

If you already have your pull request for the branch, your new commits should be part of this pull request for this branch.

You can check the changed files in your PR, and verify it.