I’d like to make the case that this RFC is getting ahead of itself. We need to focus on identifying the problems first before deciding that mandatory pre-commit review is the best solution rather than just the first solution people thought of or the quickest to implement.
As a preface, I want to be clear that I’m not against code review. In fact, I think more people should do code review more often. And I’ll freely admit that I’m human and I’ve sometimes been overly eager or overly optimistic about my changes before they went in. But requiring pre-commit review is not a change that should be made lightly. There are hidden costs, hidden risks, the gains are not guaranteed, and there are often better solutions.
As I see it, there are three core motivating concerns identified so far:
- The concern that not enough pre-commit or post-commit review is happening.
- The concern that “information silos” are developing.
- The goal that more people should be able to explain/justify/help after disruptive changes land.
I’d like to acknowledge these topics, suggest alternatives, and challenge the suggestion that mandatory pre-commit review is the “best” solution in each case – and I’d like to discuss these topics in reverse order:
First, if the community wants to decrease the short-term pain of disruptive changes, then committers need to learn that the community expects them to work harder at their communication and planning skills. Reviews – mandatory or not – are just one small step along the way of good communication/planning. If the community cannot be clear with each other about what kinds of disruptions are tolerable and which should have been planned out better, then mandating pre-commit reviews won’t magically fix the social dysfunction. In contrast, if the community is good at planning and communication, then mandatory pre-commit reviews are at best an unnecessary drag on productivity, and at worst an institutional bias against change itself.
Second, if the community is concerned about “information silos”, then more long-term communication needs to be done. Maybe that’s in the form of documentation, or a blog post, or a paper, or a talk. It really doesn’t matter. The important goal is that expertise needs to be distilled and recorded. In contrast, commit reviews – mandatory or not – are an incredibly noisy, raw, jargony, and ephemeral process. At best, one or two peers can passingly keep up with the firehose, but the community as a whole gains nothing. Making reviews mandatory doesn’t change anything in this regard, and if anything, it dilutes the value of reviewing and requesting reviews.
Third, I find the concern that not enough pre-commit or post-commit review is happening to be paradoxical. If people feel that way, then they’re actually paying attention, but they must feel that other people are not. If you feel that way, then it is your obligation to say something. You need to tell the people that are paying less attention than you do that something is going on that they should care about. It doesn’t take much. Maybe just a quick “FYI, you might want to look at this” email or a “mention” in GitHub. The important thing is that the social norm is established that we’re all expected to look out for each other. In contrast, if a engineer habitually ignores the prodding of the community, then mandating pre-commit review won’t magically convince them to care. It’ll just add friction to well behaved committers.
Dave