Quick show of hands…? How many of you have gotten into a commit war?? I think we all know what I'm talking about – some projects even have rules that exist solely to prevent commit wars from occurring (they generally take the form of limiting the frequency of commits for a given file).? These things generally start out innocently.? Some well-intentioned developer opens up a file and decides that a couple of variable names aren't as clear as they should be.? So he renames them.? Since he is already in there, he reformats a couple of lines of code to meet his personal preferences and commits the file.
The original developer is then informed of a defect in her code.? Nothing serious – a simple mistake in the index being used.? She opens the file and is confused by the naming scheme – it seems the new names conflicted with the index name, and resulted in the defect.? In addition, the code was reformatted in an odd manner.? A firm believer in “the one true brace style”, and already on edge for taking the blame for a defect introduced by someone else, she reverts back to the original check-in and reformats the entire file to fit her preference.
The first developer notes the change in the file and the loss of all his insightful and very carefully chosen variables names.? Now furious, he undoes the commit and sends a nasty note to the original developer instructing her to leave his changes alone.
Not one to take something like this lying down, the original developer begins to nit all of his code in reviews.? When he doesn't fix her complaints, she comes in afterwards and changes it anyway.
Preventing Escalation
In hind sight, commit wars are remarkably simple to prevent.? Simply by mandating a standard code format you can avoid the majority of commit wars.? In addition, it is critical to ensure that your reviews focus on real problems and not on simply stylistic or implementation preferences by the reviewer.?
Of course, the most crucial element to preventing a commit war is to create an environment and team where there is enough respect that the developers do not feel the need to look over each other's shoulders.? If no one can trust the rest of the team to do things right, then every little change is subject to enhanced scrutiny, and possible retaliation.
Finally, if every change requires a peer review, most of these problems will never make it past the review.? Too often, teams believe that only the major changes should be reviewed.? But by putting a lightweight review process in place to review every code change, you can avoid the sorts of unnecessary changes that create commit wars.? Of course, if you actually expect to get things one, you had better make sure that your reviews are quick and painless…