Preferred Github workflow for updating a pull request after code review

Question

I've submitted a change to an Open Source project on Github and received code review comments from one of the core team members.

I would like to update the code taking into account the review comments, and re-submit it. What is the best workflow for doing this? From my limited knowledge of git/github, I could do any of the following:

1. Update the code as a new commit, and add both the initial and updated commit to my pull request.
2. Somehow (??) rollback the old commit from my repository, and create a single new commit containing everything, then raise a pull request for that?
3. git commit has an amend feature, but I've heard that you shouldn't use it after you've pushed the commit outside of your local repository? In this case I have made the change on my local PC and pushed to my github branch of the project. Would this be OK to use 'amend'?
4. Something else?

It seems like option 2/3 would be nice, as the open-source project would only have one commit in their history which would implement everything, but I'm not sure how to do this.

Note: I don't know if this affects the answer or not, but I didn't make the changes in a separate branch, I just did a commit on top of the master

Answer 1

In order to update a pull request you need to checkout the same branch the pull request is from and push to it again:

cd /my/fork

git checkout master

...

git commit -va -m "Correcting for PR comments"

git push

Clean the commit history (Optional) 

In order to squash your commits together so that the repository history is clean, or yourself want to remove intermediary commits which distract from "the message" in your pull request. For instance, the commit history looks like this:

$ git remote add parent [email protected]:other-user/project.git

$ git fetch parent

$ git log --oneline parent/master..master

e6e78b8 add test case as per PR comments

eccaa56 code standard fixes as per PR comments

fb30112 correct typos and fatal error

58ae094 fixing problem

It is a good idea to squash things together so that they appear as a single commit:

$ git rebase -i parent/master 

This command will prompt you to choose how to rewrite the history of your pull request, the following will be in your editor:

pick 58ae094 fixing actual problem

pick fb30112 correct typos

pick eccaa56 code standard fixes

pick e6e78b8 add test case as per PR comments

Now just change pick to squash for any commit you want to be part of the previous commit:

pick 58ae094 fixing actual problem

squash fb30112 correct typos

squash eccaa56 code standard fixes

squash e6e78b8 add test case as per PR comments

So now if you close your editor, git will then rewrite the history and prompt you to provide a commit message for the one combined commit. Then you can amend accordingly and your commit history will now be concise:

$ git log --oneline parent/master..master

9de3202 fixing actual problem

Now push that to your fork:

$ git push -f

Counting objects: 19, done.

Delta compression using up to 4 threads.

Compressing objects: 100% (5/5), done.

Writing objects: 100% (11/11), 978 bytes, done.

Total 11 (delta 9), reused 7 (delta 6)

To [email protected]:me/my-fork.git

   f1238d0..9de3202  HEAD -> master

After this your pull request will contain a single commit, incorporating all changes previously split into several commits.

Changing the history of Public Repository (PR) can turn out to be bad for us. 

Let us see why, rewriting the history and using git push -f on a branch that, potentially, someone else has already cloned is a bad thing - it causes the repository's history and that of the checkout to diverge.

However, amending the history of your fork to correct the change you are proposing to be integrated into a repository - is a good thing. As such have no reservations squashing "noise" out of your pull requests.

Also just remember that I show the pull request as having come from the master branch of your fork, there's nothing wrong with that necessarily but it does create certain caveats such as, if this is your standard technique, only being able to have one PR open per repository. It is definitely a better idea though to create a branch for each individual change you wish to propose:

$ git branch feature/new-widgets

$ git checkout feature/new-widgets

...

Hack hack hack

...

$ git push

# Now create PR from feature/new-widgets