The issue is one of review scaling. I wrote a blog post about this a while ago[0], but the gist of it is that those clean logical units are often too small for meaningful high-level reviews of more complex work.
With complex features or refactorings, you're often in a situation where those clean logical units allow reviewers to do a good low-level review (do a check for logic corner cases, style issues, etc.) but they don´t allow a high-level review of how all the pieces of the feature work together.
IMHO the most open-source process friendly solution to the issue is to review patch series, where you can review the series as a whole for the big picture, but also dig into individual commits for the details. Building such a patch series requires an approach as described in the article.
(In closed source environments, you may get a good enough approximation of the result with a separate, disciplined software design process.)
[0] http://nhaehnle.blogspot.com/2020/06/they-want-to-be-small-t...
I don't care one lick what someone's branch history looks like. If they want to commit every day, or every hour, or after every keystroke - I don't care. All I know is that once the PR is merged, it's all going to be squashed into a logical unit so the `main` commit history will look just fine.
I could avoid creating those commits in the first place, but asking me to only commit my local changes when I have something intelligent to say about them, is a damn near impossibility for me. I basically use git to "save my work" before I try an approach to something. I reset back if it doesn't work out. Sometimes I reset back again, if the approach that didn't work out turned out to be the least worst option. I create commits to experiment with something, so that I can quickly compare back and forth between two approaches. etc. etc. etc.
I would instead say, that if you're only making commits when you have something logical to say, you're probably not using git to its fullest potential. You should really go nuts with it IMO, and only bother to sound intelligent in your commit messages once you're ready to do so, and (most importantly) you should still make commits before that happens. Git's decentralized for a reason, take advantage!
Nothing forces you to keep the messy commit messages either - keep the short commit message and make sure it's good, then delete the combined individual commit messages from the long message field, done.
About as easy as it gets. As titles are issue numbers changes are documented automatically.
Yes, that probably is a “me” problem, and I’m too anxious about it for no reason, but that’s me. But if you saw my local scratch branches you’d probably understand.
(My wip commits occasionally bitch about coworkers, for example. Or they just contain a bit too much profanity for a professional environment. Or there’s just 50 worthless “WIP” commits with no other description. You’d never know any of this from my PRs.)
That will solve your problems if you're the messy type.
You said it’s a waste of time to do this and to just squash when merging instead. I’m saying I’d rather squash first so that my PR looks clean and doesn’t contain my WIP commits. Then you respond saying “well you can just squash before your PR then”… are we going in circles?
My original comment was targeting the two supposed problems that the article is framing and the supposed convoluted (and possibly dangerous) solution with git reset.
If someone else is doing the merge and you’re unsure if they will you can preemptively squash your whole branch so there’s only one commit in the pr.
I agree! And that's what I do. But OP said that would be "a waste of time and effort", hence my reply.
Spending effort on managing git is mental effort you don't spend on solving your actual problems. By far the best experience I've ever hadeith git was: everyone works straight on the dev branch, just rebase, fix your stuff, test often, and if you're doing some multi-day work then sure, branch and think it over then merge.
That's it. That's all you need. I've had a million more problems with every attempt at making this process "clean", or "smart". Dumb was by far more efficient, more enjoyable, helped us find and fix bugs faster, and had the shortest time to market ever.
The advice I try to live by, is that however messy my work was leading up to a PR, I make sure the end result is something somebody can review without additional context. Commits should have lengthy descriptions of changes that describe the "why" and "how" of a particular change, in a way that makes it easy to digest for a reviewer. Sometimes multiple commits make sense (like if you're renaming a module/class, put that in a single commit, then put the actual code change in the next one), sometimes they're not necessary. But it's worth it to put in the effort here if it means it helps a reviewer, IMO.
besides CI only needs to test the merge commit
git bisect skip $(git merge-base main branch)
to me, squashing the tree to simplify an edge case seems needlessly radical
I assume people here have different workflows, where they work alone on small features for maybe 1-2 days and then just squash all their tiny WIP commits into one?
Yes, that's true. My original point still stands though. That commit will have a PR associated with it. You can go to it and see the details of all commits that were used for the merge commit that went into master. Not really a lot of extra work to go to the PR if you're investigating that deep.
> Even if the original branch is still floating around somewhere, that would still be an extra hassle.
If the branch deletion setting when merging a PR is selected, then only the origin branch will be deleted. If you're the one doing the PR and you don't delete your local branches you'll be able to see it locally. In the other case where is someone else's then it becomes harder, and I agree it's an extra hassle.
In my work I try to mandate PR's to be as small as possible, but not smaller. Big PR's do happen, but that and resorting to git blame or bisect happens maybe 2-3 times a year. I think the general rules for source control followed by the team are more important than the implementation details and git magic.
Dont make work upfront, that is rarely important. When it is important, find the commit and split it up (as necessary) at that singular point (instead of across all PRs). You have now cut down how much time it takes to make PRs while retaining the same end result.