8

I did some changes to a forked repo on Github and sent in a pull request. The owner had some small issues I have fixed now locally. How should I submit these fixes? When I just make a commit and push, will there be one more "fixed" commit within the pull request?

To keep it clean, I would like to rebase or amend my last commit and do a forced push, but what would happen then with the pull request? Will it be deleted automatically or just added (what I would like to happen)?

Philippe
  • 28,207
  • 6
  • 54
  • 78
Asara
  • 2,791
  • 3
  • 26
  • 55
  • Possible duplicate of [Preferred Github workflow for updating a pull request after code review](https://stackoverflow.com/questions/7947322/preferred-github-workflow-for-updating-a-pull-request-after-code-review) – Murmel Sep 24 '19 at 10:03

3 Answers3

17

A pull request is made from the reference of a branch (and not from a particular commit).

So, every change you make in the branch (that is pushed to your repository) until the pull request is merged will be reflected in the pull request.

That's a clear design made by GitHub to make contribution easier, with commits that could be added following reviews and comments. That's even a recommended workflow to open the pull request early in the development (i.e. when the work is not finished) to start early reviews or discuss about some code (the draft mode of a PR is for that).

There is no problem to amend a commit and do a --force-with-lease (that should be preferred to --force) since you are sure that you are the only one working in this branch, or you warn other developers beforehand or they know perfectly well how to handle the update (otherwise you will really bother the other developers)

Philippe
  • 28,207
  • 6
  • 54
  • 78
  • 1
    In general, I agree, but wouldn't this lead to conflicts with [pull request reviews](https://help.github.com/articles/about-pull-request-reviews)? – Murmel Sep 24 '19 at 09:18
  • @Murmel According to [this article](https://blog.adamspiers.org/2015/03/24/why-and-how-to-correctly-amend-github-pull-requests/), "When you force-push a new head for a pull request branch, GitHub handles it gracefully and automatically updates the pull request in the way you want.". Comments on modified parts of the code will be marked as outdated but other comments will stay the same. – kapex Nov 09 '22 at 21:51
0

Dont make it complicated. There is nothing wrong doing another fix commit, it will keep things simple and reduce chances of loss of code. Cheers

Shantanu Bhuruk
  • 23
  • 4
-1

Yes, I would definitely just push the new commits. Git is all about not changing the commit history.

Yeah, there probably is some hackish way to accomplish amending the changes, but there's just nothing wrong with adding another fix commit to the pull request.

Cheers.

Mladen Ilić
  • 1,667
  • 1
  • 17
  • 21
  • 3
    I totally disagree with "Git is all about not changing the commit history", because for example: `git commit --amend`, `git rebase`, `git reset` or `git cherry-pick` are all about changing the history. See also [Git Tools - Rewriting History](https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History) from the Pro Git book. There are perfectly fine situation for changing the history afterwards. Avoiding broken commits would be one example why rewriting history might be fine. – Murmel Sep 24 '19 at 13:35