5 ms·

git rebase squash as a single commit on a single main branch is the one true way. I know a lot of people want to maintain the history of each PR, but you won't

by echelon 10mo ago

git rebase squash as a single commit on a single main branch is the one true way. I know a lot of people want to maintain the history of each PR, but you won't need it in your VCS. You should always be able to roll back main to a real state. Having incremental commits between two working stages creates more confusion during incidents. If you need to consult the work history of transient commits, that can live in your code review software with all the other metadata (such as review comments and diagrams/figures) that never make it into source control.

jameshush 10mo ago
This is one of the few hills I will die on. After working on a team that used Phabricator for a few years and going back to GitHub when I joined a new company, it really does make life so much nicer to just rebase -> squash -> commit a single PR to `main`
- fc417fc802 10mo ago
  What was stopping you from squash -> merge -> push two new changesets to `main`? Isn't your objection actually to the specifics of the workflow that was mandated by your employer as opposed to anything inherent to merge itself?
hnarn 10mo ago
I completely agree. It also forces better commit messages, because "maintaining the history of each PR" is forced into prose written by the person responsible for the code instead of hand-waving it away into "just check the commits" -- no thanks.
_flux 10mo ago
Merging merge requests as merge commits (rather than fast-forwarding them) gives the same granularity in the main branch, while preserving the option to have bisect dive inside the original MR to actually find the change that made the interesting change in behavior.
- yxhuvud 10mo ago
  I wish github created automation for this flow like they have for other variants.
  - kunley 10mo ago
    But they have, with pull requests. When you merge a pull request it is done via the "subtree" merge strategt, which preserves partial commits and also does not flatten them.
    - yxhuvud 10mo ago
      There is no button to rebase, and then merge with a merge commit no-ff.
fc417fc802 10mo ago
> You should always be able to roll back main to a real state. Well there's your problem. Why are you assuming there are non-working commits in the history with a merge based workflow? If you really need to make an incremental commit at a point where the build is broken you can always squash prior to merge. There's no reason to conflate "non-working commits" and "merge based workflow". Why go out of the way to obfuscate the pathway the development process took? Depending on the complexity of the task the merge operation itself can introduce its own bugs as incompatible changes to the source get reconciled. It's useful to be able to examine each finished feature in isolation and then again after the merge. > with all the other metadata (such as review comments and diagrams/figures) that never make it into source control. I hate that all of that is omitted. It can be invaluable when debugging. More generally I personally think the tools we have are still extremely subpar compared to what they could be.
Izkata 10mo ago
> I know a lot of people want to maintain the history of each PR, but you won't need it in your VCS. Having worked on a maintenance team for years, this is just wrong. You don't know what someone will or won't need in the future. Those individual commits have had extra context that have been a massive help for me all sorts of times. I'm fine with manually squashing individual "fix typo"-style commits, but just squashing the entire branch removes too much.
- lanyard-textile 10mo ago
  Disagree! If those commits were ready for production, they would have been merged. ;) Don't put a commit on main unless I can roll back to it.
  - z3dd 10mo ago
    When your PR build takes more than an hour you'll think twice before creating multiple PRs for multiple related commits (e.g. refactoring+feature) when working on a single issue.
    - lanyard-textile 10mo ago
      That's fine and fair, but you'd better squash them then :)
eeperson 10mo ago
> I know a lot of people want to maintain the history of each PR, but you won't need it in your VCS. I strongly disagree. Losing this discourages swarming on issues and makes bisect worse. > You should always be able to roll back main to a real state. Having incremental commits between two working stages creates more confusion during incidents. If you only use merge commits this shouldn't be any more difficult. You just need to make sure you specify that you want to use the first parent when doing reverts.
noisem4ker 10mo ago
Hard disagreement. https://0x5.uk/2021/03/15/github-rebase-and-squash-considered-harmful/ https://0x5.uk/2021/03/15/github-rebase-and-squash-considere...

by echelon 10mo ago

jameshush 10mo ago

This is one of the few hills I will die on. After working on a team that used Phabricator for a few years and going back to GitHub when I joined a new company, it really does make life so much nicer to just rebase -> squash -> commit a single PR to `main`

fc417fc802 10mo ago
What was stopping you from squash -> merge -> push two new changesets to `main`? Isn't your objection actually to the specifics of the workflow that was mandated by your employer as opposed to anything inherent to merge itself?

hnarn 10mo ago

I completely agree. It also forces better commit messages, because "maintaining the history of each PR" is forced into prose written by the person responsible for the code instead of hand-waving it away into "just check the commits" -- no thanks.

_flux 10mo ago

Merging merge requests as merge commits (rather than fast-forwarding them) gives the same granularity in the main branch, while preserving the option to have bisect dive inside the original MR to actually find the change that made the interesting change in behavior.

yxhuvud 10mo ago
I wish github created automation for this flow like they have for other variants.
- kunley 10mo ago
  But they have, with pull requests. When you merge a pull request it is done via the "subtree" merge strategt, which preserves partial commits and also does not flatten them.
  - yxhuvud 10mo ago
    There is no button to rebase, and then merge with a merge commit no-ff.

fc417fc802 10mo ago

> You should always be able to roll back main to a real state. Well there's your problem. Why are you assuming there are non-working commits in the history with a merge based workflow? If you really need to make an incremental commit at a point where the build is broken you can always squash prior to merge. There's no reason to conflate "non-working commits" and "merge based workflow". Why go out of the way to obfuscate the pathway the development process took? Depending on the complexity of the task the merge operation itself can introduce its own bugs as incompatible changes to the source get reconciled. It's useful to be able to examine each finished feature in isolation and then again after the merge. > with all the other metadata (such as review comments and diagrams/figures) that never make it into source control. I hate that all of that is omitted. It can be invaluable when debugging. More generally I personally think the tools we have are still extremely subpar compared to what they could be.

Izkata 10mo ago

> I know a lot of people want to maintain the history of each PR, but you won't need it in your VCS. Having worked on a maintenance team for years, this is just wrong. You don't know what someone will or won't need in the future. Those individual commits have had extra context that have been a massive help for me all sorts of times. I'm fine with manually squashing individual "fix typo"-style commits, but just squashing the entire branch removes too much.

lanyard-textile 10mo ago
Disagree! If those commits were ready for production, they would have been merged. ;) Don't put a commit on main unless I can roll back to it.
- z3dd 10mo ago
  When your PR build takes more than an hour you'll think twice before creating multiple PRs for multiple related commits (e.g. refactoring+feature) when working on a single issue.
  - lanyard-textile 10mo ago
    That's fine and fair, but you'd better squash them then :)

eeperson 10mo ago

> I know a lot of people want to maintain the history of each PR, but you won't need it in your VCS. I strongly disagree. Losing this discourages swarming on issues and makes bisect worse. > You should always be able to roll back main to a real state. Having incremental commits between two working stages creates more confusion during incidents. If you only use merge commits this shouldn't be any more difficult. You just need to make sure you specify that you want to use the first parent when doing reverts.

noisem4ker 10mo ago

Hard disagreement. https://0x5.uk/2021/03/15/github-rebase-and-squash-considered-harmful/ https://0x5.uk/2021/03/15/github-rebase-and-squash-considere...