In his weblog, Michael Feathers writes about a company that is using the open source committer model for commercial development projects, with only a few people on the team having the ability to commit the code:
The committer model seems like a good fit for teams with wide disparities in skill set. It seems better than code review because bad code never gets into the code base to begin with, and beyond that it can galvanizes the team around a set of standards. Could there be downsides? I'm sure there are. Open source projects are voluntary. Introducing this sort of a power relationship shouldn't be done lightly. I'd expect someone to do it if, for instance, they are creating missle guidance systems, or nuclear control systems, but what about a system with many junior programmers that, say, just controls your enterprise and is responsible for millions of dollars worth of transactions a day? Would you do it then? Probably.
Ideally, I think everyone on a team should be able to commit, but it seems like it would make sense to make sure that everyone who does commit can commit responsibly, that they are able to decide whether a change is good enough to make it into the build. For some teams, it may take a while to move everyone to the point where they are able to commit.
This could be seen as a way to force code reviews by a few people who, because they are members of this special class of "committer," might feel more responsible towards the quality of the code. The committers might also be more familiar with the code, and probably more talented in general at programming. It could also be a pragmatic way to accept the fact that you can't hire only superstar programmers.
To me, however, it sounds like a process that leverages collaboration to improve quality. Articles can't get published on The C++ Source, for example, without going through Chuck Allison, the editor. Chuck is the only "committer" for TCS, and the quality of articles published there is very high. Weblogs on Artima are committed by their authors, and the range of quality of blog posts is generally good, but less consistent.
I heard once of a company that had a process by which someone had to review your code before checking. Their revision control required a different person on the team to check a checkbox before you could commit, but it could be anyone on the team. I also heard that when people were busy trying to meet their own deadline, they often just gave checked that checkbox without applying much scrutiny. If you annoint a select few with the ability of doing reviews, perhaps they would feel more responsibility towards what gets checked in and would not shortshrift the review.
What do you think of committer model idea for commercial development? Do you think moving to a committer model would benefit the projects you are working on now?
It may work for open-source projects, where the committers can act as a gateway, but in a commercial environment with lots of developers all working on various schedules, it can easily break down.
For one, the committers become the bottleneck and if they take days to get around to committing your code, the system might have already "moved on", requiring you to merge the changes into your code and then have it re-evaluated by the committer.
Second, looking at the code at the time of commit seems a bit late. By that time, the design's frozen in code and if the committer says "I don't like the design", lots of time and effort have been wasted. So without design reviews, this has high costs if the design isn't "good enough" for the committer.
Third, what if your submission requires multiple committers (because each one is only responsible for a certain area of the code)? Then you've got the above two issues multiplied.
Fourth, these aren't really code reviews unless the submitter is sitting with the committer doing the review. And even if that is done, the point of a more formal code review is to show those who are less experienced what good code should be, so they won't repeat other people's mistakes.
Finally, the committers may not only be overwhelmed by submissions, but they may also be pressured by project management to stop holding up the process.
To reduce these problems, you could make everyone a committer, where everyone would be reviewing the code being submitted in their area and the bugs/faults appearing in the code should be tracked back to the reviewer to see why it wasn't noticed. I don't know how much this would help, though, you still have the issue of people being busy and holding up reviews until the code gets stale.
> I've heard stories of the "pair committer" process at > Microsoft being quickly compromised when there was > pressure. > > Unless people buy into the quality processes, even rules > that are automatically enforced will be circumvented.
That's funny, I probably heard about that from you. I was wondering whether the problem in that pair committer set up was that anyone could review anyone else's code. If only a few people can commit, then they might feel more responsibility and not circumvent the process. Chuck Allison's certainly not going to let shoddy work get through at The C++ Source. As editor, he feels pride in what gets through there, and enforces a certain level of quality.
We're using a committer model in our company for two teams with nine members at separate locations. We were trying out various ways of organizing this model. The reason why we use it is that we want to assure that developers, who are working remotely (remote means not in the location where the operations of our Internet platform happens) have a local representative who has direct contact to the operations team. In our experience the remote team lacks all the ambiguous information floating around. Using a committer from the local team working together with a part of the remote team helps us remedy the communication gaps.
> Second, looking at the code at the time of commit seems a > bit late. By that time, the design's frozen in code and if > the committer says "I don't like the design", lots of time > and effort have been wasted. So without design reviews, > this has high costs if the design isn't "good enough" for > the committer. This is absolutely true. We tried exactly this approach where we let the submitters work on a branch for a couple of weeks and then the committer looked at it and "did not like" parts of what has been done so far. And we experienced the high costs of changing things late as well as negative effects on motivation of the submitters.
Now we let the committer work very closely together with the submitters from the beginning making sure that all the design is being done together. In that way we assure that what gets submitted only needs minor changes by the committer (formatting, naming, simplification of algorithms) but not costly design changes. This works out pretty well so far.
> > Third, what if your submission requires multiple > committers (because each one is only responsible for a > certain area of the code)? Then you've got the above two > issues multiplied. This issue we try to avoid by practicing shared code ownership and pair programming in general. Usually a committer is able to commit what ever he gets. If not he pairs up with the "expert" (not owner ;-) ) of that area while doing the commit.
> > Fourth, these aren't really code reviews unless the > submitter is sitting with the committer doing the review. > And even if that is done, the point of a more formal code > review is to show those who are less experienced what good > code should be, so they won't repeat other people's > mistakes. Our committers usually address issues they find in the code directly and discuss better ways of doing it with the submitters. For us it works out that the committers can show less experienced guys how to avoid mistakes.
> > Finally, the committers may not only be overwhelmed by > submissions, but they may also be pressured by project > management to stop holding up the process. It happened for us that the committers got overwhelmed when we did the committing after a long period of development or during testing phase. With integrating the committer very closely with the submitters, this issue has nearly completely vanished.
Another measure we took to avoid this issue is that the submitters have their own branches where they can commit what and when they want. The committer is the guy who merges these development branches into our trunk usually every second day. During testing phase we change the model to submitting patches to the committer instead of using and merging the development branches to gain more speed.
After a lot of experimenting on how the committer model could work for us we found ways now which work very well in our current setup.
The group that develops python the language has an interesting rule to keep the heat off from the main volunteer committers. I think it goes like this: if you cannot find any volunteer to review your patch, you have to review two patches of somebody else yourself. One of the people that wrote the patches you reviewed has now the moral obligation to review your patch in return.
IMHO, a better model is when 'committer' is replaced with a system architect who just helps 'submitters' make good technical design of the features/modules the latter should develop. Because 'bad code' is rather caused by 'bad design' than by, e.g., bad formatting or naming. Formatting/naming can be easily corrected whereas design re-working required a lot of time and effort for re-implementation.
I agree fully with Bill that everyone on a team should be able to commit and do so responsibly. The overall philosophy of the committer model could result in improved code quality; however, the human/power dynamics introduced by nominating an almighty enforcer could eventually result in undesirable effects on the team. Perhaps a more effective approach would be to automate the committer so it’s no longer a He or she and is in fact an "It".
Apparently Microsoft's Visual studio team system provides support for code analysis and implementation of code-check in policies. For example if one was able to implement a check-in policy against a code base which stipulates that no code can be checked in without code analysis being successfully run, this approach could make its adoption and practice a bit more palatable and also force developers to improve the quality of their code since the code base wont accept any garbage.
In essence to gain wider acceptance, more sophisticated lifecycle process tools need to be developed rather than stick wielding enforcers.
Previous Topic
