Is it OK to squash changesets of an opened pull request?
I’m in the following scenario:
- I commit some patches to a dev branch
- I push the branch on stash or github or bitbucket
- I open a merge request from dev to master
- I push some new patches that fix issues reported by reviewers
It turns out some of the patches can be squashed with the previous ones, in order to increase clarity and consistency.
- Email notifications when issues are opened in GitHub
- Remove file from github and history
- git fatal error after deleting repo
- Permission denied on git clone of my own private repo on a remote server - even though I added a new public key and all
- Importing github projects as library to existing project
- Sync work between 2 systems using git with free account on github?
Is it OK to do so (and then force push to the remote dev branch)? Is there some situations where this could break something in the merge-request-experience?
Thanks for your answers.
I’m in fact the owner of a project’s repository, not an actual user creating a merge requests. I completely understand git‘s merging and rebasing mechanisms so no need to explain git’s internals.
The question is more: as a reviewer, would you ask a submitter to squash some of its patches when it makes sense? In your experience, does it disable or break some opportunities that offer pull requests.
3 Solutions collect form web for “Is it OK to squash changesets of an opened pull request?”
Once commits have been pushed, rewriting history (which is really creating new history) is going to cause problems. After you’ve pushed your changes, your repository looks like this.
A - B - C - D - E - F [origin/master] [master] \ 1 - 2 - 3 - 4 - 5 - 6 - 7 - 8 [dev] [origin/dev]
Let’s say you decide 6 can be squashed into 3 and 7 can be squashed into 4. You do a rebase and wind up with this…
A - B - C - D - E - F [origin/master] [master] \ 1 - 2 - 3 - 4 - 5 - 6 - 7 - 8 [origin/dev] \ 3b - 4b - 5b - 8b [dev]
Even though 5 and 8 were not directly changed, they must be rewritten because their parents changed and they wind up with new IDs. Your dev branch has diverged with the one in the remote. If you push you have to force it and blow away 3, 4, 5, 6, 7 and 8. This causes problems for everyone else.
Anyone who has pulled your dev branch will now get an error when they try to push or pull. They have to
pull --force to update. If they’ve done work on top of your branch it gets complicated.
Anything which refers to commits 3 – 8 (reviews, comments, log entries, etc…) will now be pointing to the wrong place.
However, it should merge fine.
Some pull request systems deal with this better than others. Last I looked, Github will present your rebased commits as new commits and it can get a bit confusing. Because of this, I would suggest you ask the project if they want you to clean up your branch or just leave it as is.
Reviewers generally like to look at small differences between commits which you make based on their feedback. If you squash commits, then they might get confused as they have to review many changes now.
On the other hand if you do not squash commits, there would be times where you might need to re-base your branch on to of some other branch. And since you do not have squashed commit, you could potentially see merge conflicts for every commit which is painful.
No, absolutely not1.
As soon as you have pushed changes for other people to review, you have published your history. And you rarely want to edit published history. You should only squash and rebase your own personal commits that you haven’t shared with anyone.
I can understand the desire to show a “clean, perfect” history. It would be nice to push a pristine history out to the world with no errors in any of your commits. But it’s too late! You made some mistakes, and some other developers found them. So you want to hide your mistakes by rewriting history. In this world, your history can be clean, or it can be accurate, but it cannot be both at the same time.
Yes, it is very tempting to rewrite history because your are embarrassed about it.
If you don’t squash,
The reviewers can see in the history exactly how you’ve addressed their concerns.
The history can be used later as a reference, if someone raises similar concerns with other pieces of code. History is a valuable tool for learning about code!
The reviewers don’t have to review your whole pull request from scratch, because they’ve already reviewed an earlier version.
You’re trustworthy! You’re not hiding things.
If someone has pulled to review your changes, they don’t get problems next time they pull.
If you do squash,
- You get to feel like you’re the kind of perfect developer who doesn’t ever make a mistake. What an ego rush!
No squashing published history!
1: In some cases, if you made some accidental changes to formatting / whitespace, or accidentally committed build products, or something that should never have been in the history in the first place, go ahead and rewrite history (so you don’t break
git blame). So don’t take the word “absolutely” at face value.