4 ms·

Enforce squash merging to main and move on with your life. Linear history on main, individual contributors can rebase and merge or format-patch or do whatever t

Enforce squash merging to main and move on with your life. Linear history on main, individual contributors can rebase and merge or format-patch or do whatever they want on their PR branches and it doesn’t matter. There are zero downsides to this approach.

rednafi 2y ago
Yep. Neither the rebase nor merge workflow negates the benefits of squash-merging to main.
000ooo000 2y ago
>There are zero downsides to this approach. Squash merge.. zero downsides.. hard to take you seriously.
- peter_l_downs 2y ago
  What are the downsides of enforcing squash-merge to main?
  - lolinder 2y ago
    I use interactive rebase to construct a useful commit history that will communicate intent to future developers. Things like "move a file first, refactor it in a separate commit" make a huge difference when someone's coming around to view the file history later, and a squash merge removes all that information that I've meticulously encoded in the commit history. My preferred workflow on a team project is to have well-crafted commits with merge commits. The merge commits signify the wider intent and link back to the PR in GitHub, while the interactively rebased commits tell the story of the steps to get from point A to point B.
  - 000ooo000 2y ago
    Loss of utility of tools like git bisect, conventional commits, etc. Git blame is less useful when what were individual commits associated with lines are instead rolled up into a massive commit, such that each affected line is now described by a more general commit. It can encourage lazy developers to submit shit commit history in PRs knowing it's going to be squashed anyway, making PR harder.
    - bluSCALE4 2y ago
      Love those "WIP" or "" commits.
  - dahart 2y ago
    Losing granularity, obviously, no? Of course it depends on how big branches get before they’re squash-merged, but squashing can make things harder to bisect, harder to track independent semantic changes that overlap, harder to read comments & history, harder to cherry-pick small fixes… lots of things. You don’t notice the downsides until you have to manage release branches or spelunk through the history to find bugs or figure out who wrote some line of code from long back.
Snild 2y ago
There are some downsides. https://github.com/libexpat/libexpat/pull/789/commits https://github.com/libexpat/libexpat/pull/789/commits is a PR of 16 related commits. Many of them make sense individually, but are depended on by subsequent commits. Reviewing them separately makes sense. It's easier on the reviewer (and future readers) to handle multiple smaller changes, especially since each is more tightly coupled to a rationale in the commit message. Unfortunately, github's PR-focused UI doesn't really make this per-commit review as convenient as Gerrit does. Additionally, the last two commits are authored by another contributor. This metadata would be lost in a squash. Of course, each intermediate commit must build and pass tests. WIP and cleanup commits are squashed locally before final review/merge. You could argue that all of these could be separate PRs, but I think there's value in grouping them up with that final merge commit, showing what one was trying to do at a larger scale.

rednafi 2y ago

Yep. Neither the rebase nor merge workflow negates the benefits of squash-merging to main.

000ooo000 2y ago

>There are zero downsides to this approach. Squash merge.. zero downsides.. hard to take you seriously.

peter_l_downs 2y ago
What are the downsides of enforcing squash-merge to main?
- lolinder 2y ago
  I use interactive rebase to construct a useful commit history that will communicate intent to future developers. Things like "move a file first, refactor it in a separate commit" make a huge difference when someone's coming around to view the file history later, and a squash merge removes all that information that I've meticulously encoded in the commit history. My preferred workflow on a team project is to have well-crafted commits with merge commits. The merge commits signify the wider intent and link back to the PR in GitHub, while the interactively rebased commits tell the story of the steps to get from point A to point B.
- 000ooo000 2y ago
  Loss of utility of tools like git bisect, conventional commits, etc. Git blame is less useful when what were individual commits associated with lines are instead rolled up into a massive commit, such that each affected line is now described by a more general commit. It can encourage lazy developers to submit shit commit history in PRs knowing it's going to be squashed anyway, making PR harder.
  - bluSCALE4 2y ago
    Love those "WIP" or "" commits.
- dahart 2y ago
  Losing granularity, obviously, no? Of course it depends on how big branches get before they’re squash-merged, but squashing can make things harder to bisect, harder to track independent semantic changes that overlap, harder to read comments & history, harder to cherry-pick small fixes… lots of things. You don’t notice the downsides until you have to manage release branches or spelunk through the history to find bugs or figure out who wrote some line of code from long back.

Snild 2y ago

There are some downsides. https://github.com/libexpat/libexpat/pull/789/commits https://github.com/libexpat/libexpat/pull/789/commits is a PR of 16 related commits. Many of them make sense individually, but are depended on by subsequent commits. Reviewing them separately makes sense. It's easier on the reviewer (and future readers) to handle multiple smaller changes, especially since each is more tightly coupled to a rationale in the commit message. Unfortunately, github's PR-focused UI doesn't really make this per-commit review as convenient as Gerrit does. Additionally, the last two commits are authored by another contributor. This metadata would be lost in a squash. Of course, each intermediate commit must build and pass tests. WIP and cleanup commits are squashed locally before final review/merge. You could argue that all of these could be separate PRs, but I think there's value in grouping them up with that final merge commit, showing what one was trying to do at a larger scale.