5 ms·

I'm sorry, but making only "squash commits" is also a bad idea. You can have a MR with multiple independent, atomic, commits. The "real" solution is making the

by Faaak 3y ago

I'm sorry, but making only "squash commits" is also a bad idea. You can have a MR with multiple independent, atomic, commits. The "real" solution is making the developers aware of the issue and cleanup up their history before doing an MR.

orra 3y ago
Absolutely this. A series of commits dealing with different aspects (e.g. whitespace fix, minor related bug fix, necessary refactor, then the feature) is ideal. For a start, it's easy to review, and easier to disect if something breaks. When messy git history is provided (i.e. a history of the developer fixing their own code that they missed), squashing can be a reasonable fallback. But it's never the best option, IMHO.
- timost 3y ago
  I've found `git commit --fixup` and `git rebase --autosquash` to be very useful when accumulating fixes of previous commits.
  - jenadine 3y ago
    git-absorb is nice too
    - timost 3y ago
      Nice, no need to look up past commits ! Didn't know about this, I had to look it up. It's a separate project from git [0]. [0]: https://github.com/tummychow/git-absorb https://github.com/tummychow/git-absorb
alexchamberlain 3y ago
I couldn't agree more, but this is a discipline that is rare in my experience. With GitHub lacking fast forward merges, I've struggled to articulate why it's important.
- deleted 3y ago
  [deleted]
- Conan_Kudo 3y ago
  GitHub projects can be configured to force fast-forward merges.
  - alexchamberlain 3y ago
    I just re-checked github.com, and the only options are merge commit, squash commit and rebase commit. Rebase comes the closest, but none of these are fast-forward merges. Worst of all: you can't disable the green button if you don't like any of them.
    - mcpeepants 3y ago
      The "rebase" option combined with the branch protection rule of "require branch to be up-to-date" gets even closer, in my experience
      - jenadine 3y ago
        Rebase don't work if the branch you want to merge contains itself merges (that potentially fixes conflicts)
sanderjd 3y ago
Nah, automatic squashing is better. Use multiple stacked pull requests if you want multiple commits.
- lmm 3y ago
  Why? That's a whole lot of overhead for no gain.
  - iwwr 3y ago
    It's far easier for a project with many contributors. Plus, PRs and branches are cheap.
    - sanderjd 3y ago
      I don't think I ever saw one. The director of the product I worked on (comprising a few hundred people spread across a few offices and countries) sat a few "pods" over from me for awhile. He did have priority use of a 1:1 meeting room though.
      - sanderjd 3y ago
        Oops, replied to the wrong comment here. Too late to edit or delete. Oh well.
  - sanderjd 3y ago
    Hmm, I don't think separating independent units of work into separate pull requests is "overhead", I think it is good practice. And the benefit is that instead of humans spending time deciding whether or not to squash commits in some branch before merging, or being asked to do so by a maintainer, a computer just does it automatically. It's a lot like automatic code formatting; it's a big win to have tools do things automatically, rather than requiring human time and judgement and communication.
    - lmm 3y ago
      > Hmm, I don't think separating independent units of work into separate pull requests is "overhead", I think it is good practice. They're not independent, they're related. Having multiple levels of "namespace" - repo -> PR -> commit - makes things easier to navigate, just like having multiple levels of folder in your filesystem. > And the benefit is that instead of humans spending time deciding whether or not to squash commits in some branch before merging, or being asked to do so by a maintainer, a computer just does it automatically. If you demand that each PR must be a single commit you just force the human to make that decision earlier and more manually.

by Faaak 3y ago

orra 3y ago

Absolutely this. A series of commits dealing with different aspects (e.g. whitespace fix, minor related bug fix, necessary refactor, then the feature) is ideal. For a start, it's easy to review, and easier to disect if something breaks. When messy git history is provided (i.e. a history of the developer fixing their own code that they missed), squashing can be a reasonable fallback. But it's never the best option, IMHO.

timost 3y ago
I've found `git commit --fixup` and `git rebase --autosquash` to be very useful when accumulating fixes of previous commits.
- jenadine 3y ago
  git-absorb is nice too
  - timost 3y ago
    Nice, no need to look up past commits ! Didn't know about this, I had to look it up. It's a separate project from git [0]. [0]: https://github.com/tummychow/git-absorb https://github.com/tummychow/git-absorb

alexchamberlain 3y ago

I couldn't agree more, but this is a discipline that is rare in my experience. With GitHub lacking fast forward merges, I've struggled to articulate why it's important.

deleted 3y ago
[deleted]
Conan_Kudo 3y ago
GitHub projects can be configured to force fast-forward merges.
- alexchamberlain 3y ago
  I just re-checked github.com, and the only options are merge commit, squash commit and rebase commit. Rebase comes the closest, but none of these are fast-forward merges. Worst of all: you can't disable the green button if you don't like any of them.
  - mcpeepants 3y ago
    The "rebase" option combined with the branch protection rule of "require branch to be up-to-date" gets even closer, in my experience
    - jenadine 3y ago
      Rebase don't work if the branch you want to merge contains itself merges (that potentially fixes conflicts)

sanderjd 3y ago

Nah, automatic squashing is better. Use multiple stacked pull requests if you want multiple commits.

lmm 3y ago
Why? That's a whole lot of overhead for no gain.
- iwwr 3y ago
  It's far easier for a project with many contributors. Plus, PRs and branches are cheap.
  - sanderjd 3y ago
    I don't think I ever saw one. The director of the product I worked on (comprising a few hundred people spread across a few offices and countries) sat a few "pods" over from me for awhile. He did have priority use of a 1:1 meeting room though.
    - sanderjd 3y ago
      Oops, replied to the wrong comment here. Too late to edit or delete. Oh well.
- sanderjd 3y ago
  Hmm, I don't think separating independent units of work into separate pull requests is "overhead", I think it is good practice. And the benefit is that instead of humans spending time deciding whether or not to squash commits in some branch before merging, or being asked to do so by a maintainer, a computer just does it automatically. It's a lot like automatic code formatting; it's a big win to have tools do things automatically, rather than requiring human time and judgement and communication.
  - lmm 3y ago
    > Hmm, I don't think separating independent units of work into separate pull requests is "overhead", I think it is good practice. They're not independent, they're related. Having multiple levels of "namespace" - repo -> PR -> commit - makes things easier to navigate, just like having multiple levels of folder in your filesystem. > And the benefit is that instead of humans spending time deciding whether or not to squash commits in some branch before merging, or being asked to do so by a maintainer, a computer just does it automatically. If you demand that each PR must be a single commit you just force the human to make that decision earlier and more manually.