4 ms·

This is why I now love GitHub's "squash & merge"; it accomplishes so much of the ideals laid out in this "perfect commit" article.

CuriousCosmic 4y ago
I have to disagree. Mainly because I care what the contents of the branch were after the merge when I have to go back and figure out why a particular error is occurring. Mainly my workflow is: 1. Commit often with moderately useful commit titles/texts. 2. Rebase -i prior to pushing to remote. During rebase, squash/fixup together relevant commits and write succinct commit titles with useful commit texts. Then push. 3. Rebase -i again at the end of the PR to guarantee everything is coherent and that each commit that makes it in the merge at least builds and passes existing tests (i.e. that functionality before the branch isn't broken in the branch). 4. Merge into master. Merge commit should include the PR info (like PR text contents) in the merge text if at all possible. 5. Forget. Maybe move to a new git host. The years later when I identify a bug I can just: 1. Put together a reproducible example. 2. Git bisect --first-parent (only top level commits/merges) to identify the merge where the bug was introduced. 3. Git bisect the merge itself to identify the exact commit that introduced it. 4. From there I can determine the context of hows and whys behind the bug being introduced, identify a possible solution, and evaluate any process changes that could have prevented this bug from being introduced elsewhere in the project.
- manbash 4y ago
  Step 3 is problematic. When you push change requests you also need to rebase i.e. put the fixing commits right after the affected commits. Otherwise, if you only rebase once the PR is complete, then you are likely to stumble upon conflicts due to the order of affected files. It's a waste of the developer's time either way, not to mention that rebasing i.e. changing history on an (upstream) PR hurts the review experience e.g. comments on lost commits.
  - seba_dos1 4y ago
    > rebasing i.e. changing history on an (upstream) PR hurts the review experience e.g. comments on lost commits. GitLab handles it well.
  - CuriousCosmic 4y ago
    I generally agree that it's not ideal. When I say rebasing, mostly I mean going back to clean up commit texts/titles and occassionally to squash/fixup commits that at the end of the dev process don't really make sense anymore and would probably only confuse people looking back at it (like squashing/fixuping down "do a, do b, revert a, do c, revert b, do d" into "do c, do d"). I make the distinction from step 2 because while I forgot to mention it (and in my head was considering it a part of step 3), I'll generally rebase right before I open the PR which cleans up most of the noise (such as "merge master even though no relevant files were touched" commits which don't add anything meaningful). Then the rebase at the end of the PR is only really squashing down a lot of the back and forth on the code review into meaningful chunks. On Github this does kinda mess with the code snippet/suggestion stuff but since it's right before merge, all of those should be closed as resolved/implemented/etc anyways. As for it being a waste of time or not, I'd disagree. I spend maybe 5 minutes at absolute most rebasing and often only about a minute (really just git rebase -i, mark the relevant commits e/s/f, let it do it's thing poking it where necessary, resigning w/ gpg, and pushing). If the rebase is throwing nasty conflicts while working on fairly linear single-dev feature branches, odds are you are rebasing wrong. And I find this saves future me a lot of time since things generally "just work" and I have all the important history still in git.
  - b3morales 4y ago
    > changing history on an (upstream) PR hurts the review experience e.g. comments on lost commits. It does, but this is specifically a weakness of GitHub's UI, not of the process itself. It's handled better by other tools.
seba_dos1 4y ago
I believe that "squash & merge" is the most useless possible git forge feature that never makes sense to use except in cases of blatant laziness or incompetence. Its only purpose is to plaster over the mess that people who don't know how to use git well make with their branches, and relying on it is always net negative for the utility of your repository. The only case where I use "squash & merge" is when I get some trivial merge request from someone who clearly doesn't grok git well and posts tons of fix up commits. When their change would boil down to a single logical commit anyway, then sure, I can click that button instead of asking them to clean up their branch - saving time for both of us. Otherwise, there's absolutely no benefit to it that I can perceive. Every reason that I have ever heard from people using that workflow could be nullified by learning how to use git a bit better, often bringing lots of other benefits to the table. (there's one exception - "GitHub doesn't handle PR reviews well" is a somewhat valid reason, but that's a perfect excuse to look for alternatives or at least to complain to GitHub to make them finally fix their product; it's not the only forge out there, GitLab deals with branches getting rewritten during review well)
- phist_mcgee 4y ago
  Woah, that's a lot of absolute statements you've made. Pulling out the old 'it's not the tool it's you' argument as well
  - seba_dos1 4y ago
    > absolute statements In case you haven't noticed, I started the whole comment with "I believe" ;) I'm clearly not an almighty source of all truth; that's just my impression after 15 years of experience with git and many projects using it with various workflows.
- m000 4y ago
  Finally someone who gets it: history rewriting is the killer feature of git. If you don't curate your local commits before pushing, you're not using git properly. Squashed commits essentially reduce the development workflow/process to the level of previous generation VCS tools (e.g. subversion).

CuriousCosmic 4y ago

I have to disagree. Mainly because I care what the contents of the branch were after the merge when I have to go back and figure out why a particular error is occurring. Mainly my workflow is: 1. Commit often with moderately useful commit titles/texts. 2. Rebase -i prior to pushing to remote. During rebase, squash/fixup together relevant commits and write succinct commit titles with useful commit texts. Then push. 3. Rebase -i again at the end of the PR to guarantee everything is coherent and that each commit that makes it in the merge at least builds and passes existing tests (i.e. that functionality before the branch isn't broken in the branch). 4. Merge into master. Merge commit should include the PR info (like PR text contents) in the merge text if at all possible. 5. Forget. Maybe move to a new git host. The years later when I identify a bug I can just: 1. Put together a reproducible example. 2. Git bisect --first-parent (only top level commits/merges) to identify the merge where the bug was introduced. 3. Git bisect the merge itself to identify the exact commit that introduced it. 4. From there I can determine the context of hows and whys behind the bug being introduced, identify a possible solution, and evaluate any process changes that could have prevented this bug from being introduced elsewhere in the project.

manbash 4y ago
Step 3 is problematic. When you push change requests you also need to rebase i.e. put the fixing commits right after the affected commits. Otherwise, if you only rebase once the PR is complete, then you are likely to stumble upon conflicts due to the order of affected files. It's a waste of the developer's time either way, not to mention that rebasing i.e. changing history on an (upstream) PR hurts the review experience e.g. comments on lost commits.
- seba_dos1 4y ago
  > rebasing i.e. changing history on an (upstream) PR hurts the review experience e.g. comments on lost commits. GitLab handles it well.
- CuriousCosmic 4y ago
  I generally agree that it's not ideal. When I say rebasing, mostly I mean going back to clean up commit texts/titles and occassionally to squash/fixup commits that at the end of the dev process don't really make sense anymore and would probably only confuse people looking back at it (like squashing/fixuping down "do a, do b, revert a, do c, revert b, do d" into "do c, do d"). I make the distinction from step 2 because while I forgot to mention it (and in my head was considering it a part of step 3), I'll generally rebase right before I open the PR which cleans up most of the noise (such as "merge master even though no relevant files were touched" commits which don't add anything meaningful). Then the rebase at the end of the PR is only really squashing down a lot of the back and forth on the code review into meaningful chunks. On Github this does kinda mess with the code snippet/suggestion stuff but since it's right before merge, all of those should be closed as resolved/implemented/etc anyways. As for it being a waste of time or not, I'd disagree. I spend maybe 5 minutes at absolute most rebasing and often only about a minute (really just git rebase -i, mark the relevant commits e/s/f, let it do it's thing poking it where necessary, resigning w/ gpg, and pushing). If the rebase is throwing nasty conflicts while working on fairly linear single-dev feature branches, odds are you are rebasing wrong. And I find this saves future me a lot of time since things generally "just work" and I have all the important history still in git.
- b3morales 4y ago
  > changing history on an (upstream) PR hurts the review experience e.g. comments on lost commits. It does, but this is specifically a weakness of GitHub's UI, not of the process itself. It's handled better by other tools.

seba_dos1 4y ago

I believe that "squash & merge" is the most useless possible git forge feature that never makes sense to use except in cases of blatant laziness or incompetence. Its only purpose is to plaster over the mess that people who don't know how to use git well make with their branches, and relying on it is always net negative for the utility of your repository. The only case where I use "squash & merge" is when I get some trivial merge request from someone who clearly doesn't grok git well and posts tons of fix up commits. When their change would boil down to a single logical commit anyway, then sure, I can click that button instead of asking them to clean up their branch - saving time for both of us. Otherwise, there's absolutely no benefit to it that I can perceive. Every reason that I have ever heard from people using that workflow could be nullified by learning how to use git a bit better, often bringing lots of other benefits to the table. (there's one exception - "GitHub doesn't handle PR reviews well" is a somewhat valid reason, but that's a perfect excuse to look for alternatives or at least to complain to GitHub to make them finally fix their product; it's not the only forge out there, GitLab deals with branches getting rewritten during review well)

phist_mcgee 4y ago
Woah, that's a lot of absolute statements you've made. Pulling out the old 'it's not the tool it's you' argument as well
- seba_dos1 4y ago
  > absolute statements In case you haven't noticed, I started the whole comment with "I believe" ;) I'm clearly not an almighty source of all truth; that's just my impression after 15 years of experience with git and many projects using it with various workflows.
m000 4y ago
Finally someone who gets it: history rewriting is the killer feature of git. If you don't curate your local commits before pushing, you're not using git properly. Squashed commits essentially reduce the development workflow/process to the level of previous generation VCS tools (e.g. subversion).