Discipline as a Prerequisite to Critical Code Contribution
Merriam-Webster’s dictionary defines discipline as: “…”
Nah. Just kidding.
Most software teams I’ve worked on have had some degree of discipline around what code changes go into the codebase. This tends to take the form of:
- Any change should be associated with a defect or feature that is documented somewhere.
- Commit small, isolated, related changes, all associated with a single defect or feature, that can be easily rolled back.
- Test changes before committing them; avoid breaking the build at all costs.
That last one seems most obvious, but it is so critical that it has to be stated anyway. Think of it this way: If you work for me on an already functioning product, I would rather have you do NO WORK AT ALL than to break the build.
Of course, I would not pay you to do no work at all, either. But breaking the build is taking the product in the opposite direction from where we want it to go.
Some time ago I worked on a software team* where a team member had a problem with this kind of discipline. Details don’t matter too much, but the net effect was that whenever this person made a commit, the rest of the team would tense up and hold their breath as they integrated the updates. This person had broken the build enough that his/her commits weren’t trusted by the rest of the team. And even if the code would compile and run, you still didn’t trust the changes because this person would often sprinkle in little changes hither and yon that were not tied to a documented defect or feature, and often wouldn’t even call out the changes in the commit message.
(As a slight distraction here, I’ll point out how frustrating this is when the team member works in the same office as you, as cited above. When that person works remotely, it becomes MUCH MUCH WORSE.)
I discussed this with my boss one day who wasn’t sure what to do. We had a person on the team who was obviously very talented, but who was just not following good practice despite repeated requests to do so. I offered up the suggestion that I’m restating here, which is basically the following:
Disciplined software engineering should be a prerequisite to having the rights to make critical code contributions.
In other words, no matter how talented a programmer is, they shouldn’t be allowed to contribute code to critical parts of the product if they do not exhibit sufficient discipline to be there. So what are the “critical parts” and what is the “sufficient discipline”? The critical parts are the parts that, if they are broken, the software doesn’t perform it’s primary function. For a product like Firefox, the “Help” dialog is almost certainly not critical, but the HTML parsing engine definitely is. The sufficient discipline is what I outlined above, but primarily centered around not breaking the build.
So to summarize, what it means is, someone who continually breaks the build should be disallowed to commit changes directly to the critical parts of the product until they earn that right back by adopting the discipline required. Of course, if you are that free-spirited maverick, there’s a pretty strong incentive now to change your approach. You may not be paid to be disciplined, but you are paid to deliver software, and if you can’t deliver software because your approach is irresponsible, this should encourage you to get on track.
Some teams choose to approach this by building elaborate systems that enforce proper behavior. I could build a complex checkin proxy that automatically merges my changes with the latest changes in the repository, builds the product, and then runs the tests (there are tests, right?), and then only makes my commit once all of that passes acceptably. That’s nice and all, I suppose. But it seems like a lot of infrastructure, overhead, and process to impose upon the whole team, when to me it seems reasonable to expect that professional software engineers have enough discipline to do this on their own.
So if you aren’t doing this now, how do you go about changing?
Well, you’ve gotta have a suite of automated tests in order to do any of this – a “broken” build is defined as one that does not pass the automated tests. If you do not have any automated tests, it is time for you to repent, my son. I do not have time to go into this here, but you must confess your grievous sin and atone by writing an automated test and adding it to your build process. Just one test is enough to get started – but it MUST be available as a part of the normal build process. Then every time a build passes the test suite but is, in fact, broken, update your test suite.
Now that you have automated tests, insist that your developers do the following before they commit:
- Update.
- Build.
- Test.
See? It’s easy. If the tests pass, they can commit.
Now it is time for you to be disciplined. Committing code that does not pass the tests is a naughty thing to do. Do not let your developers be naughty. Take immediate action. I think I’d take the following approach:
- First time is a meeting with the boss. Go over the process. Make sure they understand the process and what happened. Personally, it seems like it would be tough to misunderstand, but hey, things happen. Don’t be presumptuous or condescending. This meeting alone might be enough to get most people in line.
- If the problem continues, require peer review before checkin.
- If the problem continues, another meeting. This time, express that all of their current assignments have been reassigned to other members of the team, and they are being reassigned work that is not critical to the functionality of the project. Let them know that you are concerned about the impact this will have on their performance, because they are expected to contribute at a higher level than what their new assignments will allow. They have to demonstrate changed behavior and commitment to discipline in order to earn the right to do the important work again.
- If the problem continues after that, it is probably performance-improvement-plan time. Or mutual-agreement-that-they-find-a-different-employer time.
