5 ms·

I ser it the other way around - why spend time on a ‘nice’ commit history in a (smallish) feature branch when you can squash merge later. I prefer one commit t

I ser it the other way around - why spend time on a ‘nice’ commit history in a (smallish) feature branch when you can squash merge later. I prefer one commit to main per feature, a long with a good description on the GitHub PR. Sometimes I’ll branch out from a feature branch for the occasional and infamous ‘get CI working’ round of 10 one-line commits though, to not make it too muddy.

ufo 3y ago
Precisely, you want to keep it about one commit per feature. I think the parent comment was worried about monster merges that squash many features together.
marcandre 3y ago
The classic example where this fails is when needing to revert something. An atomic commit for the migrations + some atomic commits for the implementation mean you can easily revert the implementation, and leave the migration intact (as should be) and add a reverse migration.
gwright 3y ago
> why spend time on a ‘nice’ commit history in a (smallish) feature branch when you can squash merge later. Several reasons: * facilitates much better code review discussions * enables use of git bisect to locate bugs * allows for informative commit messages associated with the changes * communicates clearly to future self about why changes were made
- u801e 3y ago
  Another good reason is that having a small commit that changes just one thing is a lot easier to revert without encountering conflicts, even after other features have been committed to the main/master branch.
- toast0 3y ago
  > * enables use of git bisect to locate bugs This is really only viable if each intermediate commit on a development branch is intended to be bug free. If that's the standard you and your team work with, that's fine, but it's not usually my standard; in a development branch, I may commit things that don't even compile, let alone work, if it's a good point to commit.
  - bonzini 3y ago
    The point of the parent comment is exactly that you should clean up the history before merging to a public branch, so that you can use bisect, even if so far you had wip wip doh wip as the commit messages. The way to get there is to have a mix of proper and wip commits.
    - WorldMaker 3y ago
      If merge points are your "known good" points anyway you can just use the powers of the git dag and `git bisect --first-parent` in your main branch to just bisect the merge points. There's no need for rebase/squash and you still get useful git bisect results.
      - bonzini 3y ago
        All commits are good points and potentially useful points. Was the bug in the refactoring? In the feature itself? In the resolution of merge conflicts? You can only answer if you don't squash, and it becomes easier to fix the bug if you know the answer.
        WorldMaker 3y ago
        Sure, but also no one particularly wants to CI every commit inside a PR, so there is a usefulness in `git bisect --first-parent` as the "first pass" of known CI points (merge commits presumably from PRs) to find the "PR that introduced the problem" and then drill down into every smaller commit to see if you can get additional bisect information (from commits that may or may not have passed CI in the first place in development work-in-progress).
- bogeholm 3y ago
  > facilitates much better code review discussions Hmm, I usually mark PR’s as draft until ready for review, and then I expect the discussion to be about the current state, not a previous intermediate state. Easiest with small PR’s. > enables use of git bisect to locate bugs Interesting. I know _of_ git bisect, but haven’t used it as part of my workflow. Have you found it useful to bisect commits on a feature branch (which, presumably, represents unfinished work)? > allows for informative commit messages associated with the changes I find using the PR title and accompanying info in GitHub or similar to be quite informative - that should convey the purpose of the change. > communicates clearly to future self about why changes were made See above. Perhaps we work differently, but I find it clearer to read a git history where each commit represents a single, complete feature/fix/refactor instead of intermediate steps.
- sangnoir 3y ago
  There are a multitude of Git workflows, and opinions on what the basic unit of change is: for some, a feature is atomic, so squash-merging feature branches is perfectly natural. > facilitates much better code review discussions This can be done while adding code to the feature branch > allows for informative commit messages associated with the changes I'm assuming you consider individual commits to be the basic unit of change? This isn't always the case. Some products are not amenable to adding features fractionally > communicates clearly to future self about why changes were made You can do that with a squash-merge too! I've noticed people who work on an evergreen deployment can afford to work on a very granular, commit-level. However, if you have to support multiple production branches concurrently and often have to cherry-pick features and fixes across them, features will naturally become the basic unit of change you will find yourself gravitating towards, and will liberally use squash-merging just to keep your sanity.
folmar 3y ago
If things are heading towards a single commit an amend commit works fine for me. If I need a previous state I just get it from reflog.
throw555chip 3y ago
> why spend time on a ‘nice’ commit history in a (smallish) feature branch when you can squash merge later. Agreed, it has been standard at most shops I've worked in the past 8-10 years.
m000 3y ago
> I ser it the other way around - why spend time on a ‘nice’ commit history in a (smallish) feature branch when you can squash merge later. Squash-merge is a scourge. I've seen squash merged commits 30 lines long ("try 15", empty line, "try 14", empty line...). I'm not even sure if you can do anything about such commits because squash-merge is a github/gitlab thing. So, I'm not sure if there are hooks to block it via a commit message linter. And I've seen people going through some intense mental gymnastics to justify avoiding squashing locally, writing a proper commit message and then merging.

ufo 3y ago

Precisely, you want to keep it about one commit per feature. I think the parent comment was worried about monster merges that squash many features together.

marcandre 3y ago

The classic example where this fails is when needing to revert something. An atomic commit for the migrations + some atomic commits for the implementation mean you can easily revert the implementation, and leave the migration intact (as should be) and add a reverse migration.

gwright 3y ago

> why spend time on a ‘nice’ commit history in a (smallish) feature branch when you can squash merge later. Several reasons: * facilitates much better code review discussions * enables use of git bisect to locate bugs * allows for informative commit messages associated with the changes * communicates clearly to future self about why changes were made

u801e 3y ago
Another good reason is that having a small commit that changes just one thing is a lot easier to revert without encountering conflicts, even after other features have been committed to the main/master branch.
toast0 3y ago
> * enables use of git bisect to locate bugs This is really only viable if each intermediate commit on a development branch is intended to be bug free. If that's the standard you and your team work with, that's fine, but it's not usually my standard; in a development branch, I may commit things that don't even compile, let alone work, if it's a good point to commit.
- bonzini 3y ago
  The point of the parent comment is exactly that you should clean up the history before merging to a public branch, so that you can use bisect, even if so far you had wip wip doh wip as the commit messages. The way to get there is to have a mix of proper and wip commits.
  - WorldMaker 3y ago
    If merge points are your "known good" points anyway you can just use the powers of the git dag and `git bisect --first-parent` in your main branch to just bisect the merge points. There's no need for rebase/squash and you still get useful git bisect results.
    - bonzini 3y ago
      All commits are good points and potentially useful points. Was the bug in the refactoring? In the feature itself? In the resolution of merge conflicts? You can only answer if you don't squash, and it becomes easier to fix the bug if you know the answer.
      - WorldMaker 3y ago
        Sure, but also no one particularly wants to CI every commit inside a PR, so there is a usefulness in `git bisect --first-parent` as the "first pass" of known CI points (merge commits presumably from PRs) to find the "PR that introduced the problem" and then drill down into every smaller commit to see if you can get additional bisect information (from commits that may or may not have passed CI in the first place in development work-in-progress).
bogeholm 3y ago
> facilitates much better code review discussions Hmm, I usually mark PR’s as draft until ready for review, and then I expect the discussion to be about the current state, not a previous intermediate state. Easiest with small PR’s. > enables use of git bisect to locate bugs Interesting. I know _of_ git bisect, but haven’t used it as part of my workflow. Have you found it useful to bisect commits on a feature branch (which, presumably, represents unfinished work)? > allows for informative commit messages associated with the changes I find using the PR title and accompanying info in GitHub or similar to be quite informative - that should convey the purpose of the change. > communicates clearly to future self about why changes were made See above. Perhaps we work differently, but I find it clearer to read a git history where each commit represents a single, complete feature/fix/refactor instead of intermediate steps.
sangnoir 3y ago
There are a multitude of Git workflows, and opinions on what the basic unit of change is: for some, a feature is atomic, so squash-merging feature branches is perfectly natural. > facilitates much better code review discussions This can be done while adding code to the feature branch > allows for informative commit messages associated with the changes I'm assuming you consider individual commits to be the basic unit of change? This isn't always the case. Some products are not amenable to adding features fractionally > communicates clearly to future self about why changes were made You can do that with a squash-merge too! I've noticed people who work on an evergreen deployment can afford to work on a very granular, commit-level. However, if you have to support multiple production branches concurrently and often have to cherry-pick features and fixes across them, features will naturally become the basic unit of change you will find yourself gravitating towards, and will liberally use squash-merging just to keep your sanity.

folmar 3y ago

If things are heading towards a single commit an amend commit works fine for me. If I need a previous state I just get it from reflog.

throw555chip 3y ago

> why spend time on a ‘nice’ commit history in a (smallish) feature branch when you can squash merge later. Agreed, it has been standard at most shops I've worked in the past 8-10 years.

m000 3y ago

> I ser it the other way around - why spend time on a ‘nice’ commit history in a (smallish) feature branch when you can squash merge later. Squash-merge is a scourge. I've seen squash merged commits 30 lines long ("try 15", empty line, "try 14", empty line...). I'm not even sure if you can do anything about such commits because squash-merge is a github/gitlab thing. So, I'm not sure if there are hooks to block it via a commit message linter. And I've seen people going through some intense mental gymnastics to justify avoiding squashing locally, writing a proper commit message and then merging.