Comment by sanitycheck | Hacker News Reader

Comment by sanitycheck | Hacker News Reader

Parent Full thread

I like seeing small commits, mine and other peoples. It means when something's broken I can easily narrow down the cause to a single small change and either unbreak it myself or point the appropriate team/person at it, making it a quick and easy fix.

If squashing happens then all I can immediately say is "something in this 600 line changeset over 12 files for Feature X broke it". The bug report for that one is going to be more vague, get allocated more story points, and maybe stay in the backlog for several sprints (or forever).

If people are pushing their feature branches and not deleting them that makes life a bit better, but generally people who want their git history to be "clean" and squash commits also want to get rid of old branches.

Someone might say code review should have caught it. Code review almost never catches actual bugs. Or tests? Tests only test stuff that the author expected to break.

The main things I try to ensure are that every commit is small, every commit has a useful message, and no commits should break the build.

Squashing and rebasing doesn't have to produce one gigantic commit. It can take, say, 20 "WIP" commits (many of which might not even build), and organize them into 5 commits that constitute logical units with descriptive messages. But raw WIP commits themselves are rarely so organized, unless you invest a lot of time and effort into that while coding.

Personally, I prefer my coding to be free of such distractions, and to use SCM facilities as a scratchpad for fast iteration on the code (the ability to easily reverse changes etc); this results in many commits that don't make sense in the final pull request / code review.

I've heard of people doing this, but never seen it in real life. Do each of your "logical unit" commits build and run properly on their own? I'm interested in the concept, if there's a tool/process which makes that easy to achieve - but it sounds like possibly more work than just making good small commits to begin with.

I also do this. I prefer the various checks all pass, although I am likely to temporarily break lint when "make the change" and "make it pass lint" are independently obviously correct but look messy when composed. It is very rare (and I'll loudly call it out in the commit message) that a similar thing applies to types or tests and I'm more likely to actually disable the relevant test than suppress lint I'm fixing in the next commit in the same PR (although on reflection maybe I should be doing that).

The bulk of my PRs are still one small commit, but when a feature gets larger I find that "what's the next thing to try to solve the problem" isn't always the same question as "what will make this most readable for a reviewer." I tend to address presentation of the commits in the PR at about the same time I'm addressing readability of the resulting code, as they wind up being related concerns (though not exactly the same thing).

It really depends on how you choose to define the granularity. I prefer my commits to all be buildable at least, and ideally also pass the tests (so new code + new tests for it should be in the same commit); but that is subjective and there are valid arguments for smaller commits that are not necessarily isolated like that.

I can't think of any special tools you'd need for this? A git history visualizer with the ability to easily diff and squash ranges helps, but this is nothing special for pretty much any modern IDE. At the end of the day, you just look through the WIP branch history and identify chunks that span multiple commits but logically represent a single change towards some goal.

"I prefer my commits to all be buildable at least, and ideally also pass the tests (so new code + new tests for it should be in the same commit); but that is subjective"

In my view, it is not subjective. All commits locally must pass unit, integration and e2e tests, and be locally tested by the developer, every...single...one.

Right, OK - pretty manual then. I was envisaging some sort of n-way diffing tool which simulates several staging areas and changes could be moved around line by line, maybe a button to run a build + tests for each in parallel. Gets difficult as soon as code in one of the middle commits needs a bit of extra tweaking to work though, I suppose.

I'll have to try it sometime, I have a feeling it wouldn't normally turn out too differently to what I end up with anyway. I do think all commits should build and run properly - otherwise git bisect and similar processes stop being useful.

Reply on news.ycombinator.com