0

I sent a simple pull request to a Github project. A maintainer suggested a different approach. How can I adjust my pull request with the new approach?

I understand that it's normal to add commits to an already-existing pull request, but here I'd basically retract the entire first commit. What is the standard approach?

Anna
  • 2,645
  • 5
  • 25
  • 34
  • You can add new commits, it's ok. If you want you can squash(https://stackoverflow.com/questions/5189560/squash-my-last-x-commits-together-using-git) them into single commit when the PR will be ready – Max Jun 12 '21 at 20:20
  • Thanks @JonhyBeebop! Can I squash the branch even though a pull request for the branch has already been open on upstream? – Anna Jun 12 '21 at 20:23
  • @JonhyBeebop I was reading the linked question and a highly-upvoted comment warns "Take care to only squash local commits. Never touch pushed commits!" – Anna Jun 12 '21 at 20:26
  • 1
    Close the pr and start over? – matt Jun 12 '21 at 20:29
  • @Anna Yeah, you can squash pushed commits. Add push new (squashed commit) with `-f` (force) flag. And yes it's kind of scary because you can delete something you don't want to delete. But I still think you should just add new commits and do not think about previous. It is okay when during development the whole concept of your solution changes – Max Jun 12 '21 at 20:32
  • What people have said, just detailing that you can also drop your existing commit without squashing by git reset-ing or in an interactive rebase. Regardless there is then no harm in force pushing to your own branch or fork/branch (of course, provided you only rewrite history on your own unmerged commits). But simply adding more and then squashing (or fix-uping even) them into one at the end sounds the best, to me. – firmament Jun 12 '21 at 20:39
  • @JonhyBeebop Alright, thank you! I just want to make the maintainer's job as easy and clear as possible. – Anna Jun 12 '21 at 20:39
  • @Anna Github provides builtin feature `squash and merge`, so if maintainer doesn't want to see your development history he can just use it. So there is no additional job for maintainer – Max Jun 12 '21 at 20:42
  • @JonhyBeebop Aha, that's great to know! Thank you! – Anna Jun 12 '21 at 20:43

2 Answers2

0

If you want to replace the exiating commits, You can force push to the branch associates with the pull request.

plugwash
  • 9,724
  • 2
  • 38
  • 51
0

You have basically three choices.

  • Close the PR and start over. Begin a new branch with a different name at the point where this one started (or at the current end of the upstream main branch).
  • Keep the current PR and push new commits to it. In that case I would recommend starting with a single revert commit that undoes your entire initial attempt, as described in some of the answers to How to revert multiple git commits?
  • Keep the current PR and rewrite history by rebasing /squash to hide your first attempt, as suggested in some of the comments. I am strongly against that idea. Don't rewrite pushed commits.
matt
  • 515,959
  • 87
  • 875
  • 1,141
  • 2
    While "Don't rewrite pushed commits" is good advice in general I think branches that only exist for the purpose of making pull requests are an exception as the alternatives are worse. Closing the pull request and starting a new one breaks up the discussion on the feature. Keeping the rejected approach in history adds unnecessary mess to the history of the main branch when the pull request is merged. – plugwash Aug 04 '21 at 16:26