I agree Git Blame is useful, but squash on merge is great, and having atomic commits for whiteline changes is overkill.
Looking forward to that fleshed out blog post on git
FWIW this is something you see commonly on mailinglists. Each line you modify in a given commit is assumed to be specifically related to the change described in the commit description. It's just easier for reviewers when you break whitespace changes or non-semantic text changes out into separate commits so that they don't have to try and figure out which lines are semantic changes and which aren't.
Given that people who work with patchsets tend to do a lot of rebasing in their workflows, they are generally familiar enough with rebasing to just split out a set of changes from a given commit into two commits in a minute or less.
1. Write a bunch of code and just scratch it out, stashing in your `FEATURE` branch until the feature is done.
2. Rebase that `FEATURE` branch into sane commits that each accomplish an atomic substep in the implementation of your feature. Each discrete commit/change should build and pass tests.
3. Open your code for review, either via a PR or by submitting the patchset to a mailing list.
4. Implement requested changes as a `FEATURE-v2` (or `FEATURE-vX`) branch.
5. Rebase your `FEATURE-v2`/`FEATURE-vX` branch to clean up your changes and get them to roughly line up with your "final" commits from your previous `FEATURE` branch.
6. Submit your new patchset revision to the mailing list in reply to your last patchset revision. Or if you are using pull requests, change the merging branch from `FEATURE` to `FEATURE-v2`.
Then cycle back to step 4-6, rinse repeat until everyone is happy. Then you merge in the final `FEATURE-vX` branch.
This leaves you with a history where each individual commit is a useful, descriptive, and fully functional change to the codebase but also with a merge for the full feature at the top. That's important because git tooling actually can iterate over all commits or over only top level commits without traversing into merges.
Then it's way easier to identify which feature introduced the issue in question and you can easily peek into the feature's individual commits to understand each discrete change and exactly what the intent was.
OP is wrong about commit messages ("fixp" is fine, most of the time you're not going to read the commit message) but right about everything else on the git side.
But realistically the point of good commit messages is similar to the point of code formatting standards, which is to prevent the https://en.wikipedia.org/wiki/Broken_windows_theory on your codebase. If everything you do is held to a high standard, you'll hold everything you do to a high standard. If the organization stops doing that quality stays the same for a while but eventually slips and slips and slips. It is important to always enforce slightly more quality than you have now to keep the momentum in the other direction.
Sure. But the point is to get away from doing those "complicated arduous change"s, and split them up into much smaller pieces. Being able to commit without breaking your flow helps a lot with that.
> But realistically the point of good commit messages is similar to the point of code formatting standards, which is to prevent the https://en.wikipedia.org/wiki/Broken_windows_theory on your codebase. If everything you do is held to a high standard, you'll hold everything you do to a high standard. If the organization stops doing that quality stays the same for a while but eventually slips and slips and slips. It is important to always enforce slightly more quality than you have now to keep the momentum in the other direction.
This is a purely circular argument. "It's important to have high quality commit messages so that you will have high quality commit messages". It really isn't.
It also means that when you git bisect, you know exactly which PR introduced the issue. The exact commit that did so isn't as important, because there's no way to ensure that it's safely revertible.
Hardly. You know that master-before-that-commit passed CI, sure, but you don't know that reverting it isn't going to interact badly with subsequent changes or subsequent stored data.
> It also means that when you git bisect, you know exactly which PR introduced the issue.
Finding the PR from the commit is a one-liner. Github will even show you right there on the commit's page.
> The exact commit that did so isn't as important, because there's no way to ensure that it's safely revertible.
In my experience reverting a single small commit is a lot safer than reverting a full PR that may have e.g. added a value to an enum that's stored in the database, and when you have a culture of small, atomic, well-separated commits then each commit is likely to pass tests etc.. Also if the commit is a few lines then you're much more likely to understand why the breakage happened, rather than blindly reverting a big PR because your test passes before but not after, which is good for safety. Of course you need to test the revert but you always need to test the revert.
This is why it falls apart: culture is really hard to scale. If you don't have mechanisms in place to enforce this behavior, then people are going to drift. This is extra true for commit behavior, because it doesn't matter how a PR is broken down into commits; CI doesn't run on every commit. So when things Go Wrong, it is very unlikely that the buggy commit had CI run on it and the commit before, so it's probably not atomic.
But the PR always is! PR's pass CI, and the main branch does too. Sure, not every PR is safe to trivially revert, but reverting a PR will always put the codebase in a known previously good state that passed CI.
You can run CI on every commit if you want, or have a pre-commit hook to run tests, or just accept that it won't be 100%. In my experience running the build+test when you commit is self-enforcing, because otherwise you end up hitting a failure at the CI stage and that slows you down.
> So when things Go Wrong, it is very unlikely that the buggy commit had CI run on it and the commit before, so it's probably not atomic.
If you allow frequent commits then commits are more likely to end up atomic. But also the worst case is that you fall back to what you had before - your bisect script should already skip commits that fail in-tree tests, so in the worst case your automated bisect reports that the break happens somewhere in the range of commits that's one whole PR. Usually you do better than that.