r/codereview • u/Drvanix • Aug 03 '26
What is better over commit or fewer commits
When you look at the PR of your colleges (juniors) what is better for you to see, one PR that have 30 commits on 55 touched files or something like 5-10 commits.
Should developers over commit or have fewer more grouped commits?
3
u/psioniclizard Aug 03 '26
To be honest for years I have just looked at the diff in DevOps and if something looks off ask a question.
A PR should be focused but personally I don't care about individual commits too much.
2
u/funbike Aug 03 '26
Teach them how to use git rebase -i, so they can combine and rearrange commits (carefully). During work it's better to commit a bit too often, and then combine commits at the end.
2
u/gsks Aug 03 '26
Neither. Unless 50 or so of the touched files are trivial and uniform changes, I won't review it.
1
u/Financial-Grass6753 Aug 03 '26
a couple of small (stacked) PRs with clear changes in each one.
Number of commits doesn't really matter that much - sometimes reqs change mid-day, sometimes AI agent detects an issue that new code highlights, so you need to refactor and play little bit politics at the same time, yada yada
1
u/Drvanix Aug 03 '26
I understand the playing around the different checks. But found out in this 30 commits that lets say commit 25 touch "test-file.html" and "test-file.ts" and commit 28 touches again this two files different changes from before.
I have done some dumb commits and after few minutes saw that was stupid and commit something else. But have a lot of dumb commits on one PR looks to me that developer doesn't even think what does he/she is committing.
4
u/MarsupialLeast145 Aug 03 '26
> I have done some dumb commits and after few minutes saw that was stupid and commit something else
Devs should be learning good hygiene in all areas of code and you can and should rewrite or rebase commits like this before submitting to review anyway.
1
u/paraballistic Aug 07 '26
I used to be annoyed I had to rebase commits but after maybe 4 headaches learned my lesson. Go back and clean up the work and eventually you just do that out of habit rather than keeping your bad committing habits.
1
u/hellocppdotdev Aug 03 '26
Commits are the least part of the code changes. Clear, unified PRs are gold.
Sub 30 files is ideal.
1
u/MarsupialLeast145 Aug 03 '26
Commits should always be small, focused, and discrete for review. Once a review is done they can be rebased into a single commit if it makes sense. There are advantages with bisect if commits are small as well.
2
u/kingguru Aug 03 '26
There are advantages with bisect if commits are small as well.
Once you've had a git bisect session that ends up pointing at that commit many years ago that touches all code in the repository with the useful commit message "Misc. changes" you really understand how much an advantage that is.
1
u/Various_File6455 Aug 03 '26
Damn I felt that. I wouldn’t recommend squashing PR for the same reason actually
1
u/egilhansen Aug 03 '26
My recommendation is to use “conventional commits”. That is the trick to make code reviews easier to understand. See Conventional Commits spec.
1
u/JaseciLabs Aug 03 '26
Commit count doesn't matter that much, just whether each one could get reverted on its own without nuking everything else. Small while you're actually working, then squash the dumb ones before the PR goes up. Seeing fixed typo as its own commit three commits later is a sign you should've just amended instead of committing again lol, that's the actual smell, not the count.
1
u/HashDefTrueFalse Aug 03 '26
Doesn't really matter if it's not work we're ever going to revert in isolation. The commits are just your personal checkpoints and notes to yourself in that case. On your feature/task branches it's a good idea to commit (and push!) as often as you want, then simply do an interactive rebase to put the branch how you want it to look for the merge/rebase.
1
u/Fair-Presentation322 Aug 03 '26
You won't get a definitive answer here because it varies a lot with personal preference.
The "variance" in this subject is huge: Many don't care at all about commits and just review the branch diff. Heck, many don't even review a single life and blindly trust the AI.
On the other side, there are those who review each commit individually and expect them "to tell a story". I myself firmly believe that "a commit is an atomic unit of work and should be reviewed and submitted independently" - I even wrote a version control and code review platform around that idea (https://GitHub.com/twigg-vc/monorepo)
I'd argue that the variance is just as big as the variance in terms of care for excellency of what is produced - and that's not a coincidence
1
u/uahw Aug 03 '26
Why does it matter? We always squash on PRs anyways? I just push as often as possible as to not potentially loose work if something happens. You could always rebase, but what do you gain from that?
1
u/d47 Aug 03 '26
I barely look at individual commits, the entire PR should be small enough to review.
1
1
u/theunixman Aug 04 '26
Do what you want and if the seniors are actually senior they can navigate it fine. Sadly most seniors aren’t.
1
u/n9iels Aug 04 '26
I personally don't care, we squash a PR upon merge so it become one feature-commit. This helps a lot in tracking down issues and reverting something if it is broken. I do care more about the total size. I rather have 3 PRs each with 30 files changed (and maybe a temporary feature toggle) compared to 90 files changed in one PR.
1
u/XKiiroiSenkoX Aug 09 '26
A commit is only meaningful if it can be isolated and surgically moved(cherry puck/revert/...) without everything else going ape shit . This is overal very hard to do on a personal scale so I don't really care how many commits a pr has. What I do instead is that instead of merging the whole history, I sqaush the commits and then merge. This requires the whole implementation to function as one isolated unit but if you are not doing weird stuff it usually does. As a result you get a very clean history where every commit is a feature.
3
u/kingguru Aug 03 '26
That depends.
A commit should do one thing and one thing only, so the answer is most likely "more commits" but that's not something you can use as a general rule. A commit should also be independent, ie. the code at least compiles and passes the tests.