Software Engineering
programming-practices version-control git
Updated Mon, 23 May 2022 12:43:24 GMT

Is it a good practice to split one commit into two, where one is content change, the other is style(indent) change?


Suppose I am to make a commit that wrapped a section of code inside another thing, so the section came to have 1 more level of indent. In diff it will show delete 100 lines, add 100 lines though all it changed is indent level, and this is very noisy noise.

If I split one commit into two, the first one is the real code change(deliberately not add indent), the second one consists of only indent changes. This scheme can greatly separate the signal and noise (content change and style change), the diff will be much easier to read. Is it a good practice?




Solution

I think that's taking it too far. Putting functional changes and stylistic changes in different commits is good practice as it keeps the commits focused and comprehensible. However, indentation is a quasi-functional aspect of code -- it signals what level of nesting we are at in any block, which says a lot about how the code is going to be run. So I would disagree that we have two changes "nest this block" and "indent this block", rather you have one change "nest this block" that by the conventions of code formatting requires "indent this block".

The block being part of the change is in my mind a good thing as it makes it clearer what changed. If I had two separate commits as you propose, one could easily look at the first commit and say "okay, they added an if statement, but what the heck is the body of that statement?" and then "okay, they indented this block of code, why on earth did they do that?" Whereas with one commit there's no confusion.

I think this is also a case where you might need to use your tools better. Where I work, our cr review software makes clear when part of a diff is caused by changes in whitespace, and one can use git to also make this distinction (e.g. with git diff -w).