A Tip for Closed Source Software Houses: Hold off on giving Commit Access

In the preface of Kent Beck's fantastic "Extreme Programming Explained" book he spells out the core principles of XP:

• If code reviews are good, we'll review code all the time (pair programming). 
• If testing is good, everybody will test all the time (unit testing), even the customers (functional testing). 
• If design is good, we'll make it part of everybody's daily business (refactoring). 
• If simplicity is good, we'll always leave the system with the simplest design that supports its current functionality (the simplest thing that could possibly work). 
• If architecture is important, everybody will work defining and refining the architecture all the time (metaphor). 
• If integration testing is important, then we'll integrate and test several times a day (continuous integration). 
• If short iterations are good, we'll make the iterations really, really short—seconds and minutes and hours, not weeks and months and years (the Planning Game). 

As someone who is, not only a programmer, but responsible for making sure the correct products make it out the door on time, only three of these are critically important to me:

• If code reviews are good, we'll review code all the time. 
• If testing is good, everybody will test all the time.
• If integration testing is important, then we'll integrate and test several times a day

The others are important, but for different reasons and we'll touch on them later in this blog. 

The list is very similar to a story Anthony Robbins tells in "Awaken The Giant Within". Someone asked him "How do you become a great public speaker?" and he essentially said "Talk in public all the time". Poor testing and Integration are, without a doubt, the two things under our control that will make a software project go off the rails. Crappy code is the other. Test all the time. Integrate all the time. Read the code all the time. But how can you do this effectively? TDD and Continuous Integration are well solved problems. The real challenge is getting others to look at your code.

Many suggest Peer Programming. Most developers I know don't like it or can only handle it in small doses and management often have a hard time rationalizing it. Personally I'm not a fan. I find too much time is lost because the development thought process hasn't fully gelled yet. It's gratuitous programming. But the motivation is sound. In the Open Source world, Linus' Law states "With Enough Eyeballs, All Bugs Are Shallow." We need to have visibility on our code and, I think, Eric Sink puts it most succinctly: "Read the diffs!" This is a great way to read the code after the developer has had time to get their thoughts together. My issue is that it happens too late. Once the code is in the revision control system you have to make immediate decisions on how to rectify any problems you find during the review. Revert the code and repair or wait for the repair with broken windows in your source?

I prefer Jacob Kaplan-Moss's perspective on granting commit bits ... don't. If lower level developers cannot perform commits it forces the higher-level developers (the ones trying to enforce the religion of the project) to review their code. They are looking for your use of coding style, of enforced idioms and documentation. If ... IF ... you can consistently code to the norms of the company THEN you are granted commit bits to the revision control system. You can then review other developers code, merge their patches and mentor them on what is important. So, I would make a slight variation to Eric's article and change it to "Read the Patches". 

For the corporate software houses that never let their source outside the door, you can still have the benefits of the open source world by making code review a mandatory part of your development process. No projectors, no print outs, no meeting rooms. Just turn off commit access. Right now ... how many people on your corporate development project should have their commit bits revoked? 

Revoke and Review is my new mantra.