While pairing with developers, I have often noticed that they have a tendency to periodically do a mixed format.
What is a mixed format?
Now I have no idea whether this is the official term, but here is what I mean when I say, “mixed format”. A mixed format is when a developer, working on some code, comes across some other code that is not formatted as per the project’s conventions. This code could span a few lines, or in worse cases, a whole file. The developer immediately invokes his editor’s format command, and formats the offending lines, or the whole file. With a satisfied smile on his face, the developer moves on to complete whatever work he was originally tasked to do. He then creates a commit that includes:
- the work he was originally tasked to do, and
- the formatting that he set right.
What’s wrong here?
Now, from the point of view of clean code and team work, formatting is not wrong. However, I do not recommend crafting a commit that mixes both format changes and logic changes, when the following conditions hold true:
- The format changes are not related to the actual lines that the logic change encompasses
- The format changes are more than logic changes
Why? Consider what happens when the developer goes ahead and checks in his code to the VCS. Other developers reviewing his commit immediately notice that the commit’s code changes are too many - this results in an impression forming in the reviewer's mind which can range between “Wow, this is a large commit. I need to go line by line” to a feeling of just giving up. With inexperienced or bored developers, it is usually the latter.
Also consider what happens when sometime in the future, a developer realizes that your commit introduced a line that causes a bug. In order to ensure a clean fix, he opens your commit with the intention of understanding what you intended to fix. And he arrives at the same realisation - your code changes are too many. Without any choice, he is forced to go through each line to understand what it does. Imagine his frustration when most lines turn out to be formatting changes, and hidden among the formatting changes is the actual change he’s looking for.
The lesson here is to avoid large formatting changes mixed with logic changes. Prefer to stick to formatting only those lines where your feature/bug also demands a change. If you can’t avoid this, then make two commits - one for the feature/bug changes, the other just for formatting changes.
This is only a recommendation, not a rule
As soon as you read this, please don’t fire up the comments editor or your blog editor to write a comment/blog about why I am wrong. I understand this is basically a Considered Harmful essay, and I know that Considered Harmful essays are considered harmful. With that in mind, I’ll only say that the above is a recommendation, not a rule. When making such a commit, please do think about how a future you would feel if you came across such a commit, and how you’d react.
