Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

All programmers have their style of programming. But some of the styles are let’s say... let’s not say. So you have code review to try to impose certain rules for good design and good programming techniques.

But most of the programmers don’t like code review. They don’t like other people criticizing their work.

Who do they think they are to consider themselves better than me and tell me that this is bad design, this could be done in another way. It works right? What is the problem? This is something they might say (or think but not say which is just as bad if not worse).

So how do you make people accept code review without starting a war?

How can you convince them this is a good thing; that will only improve their programming skills and avoid a lot of work later to fix and patch a zillion times a thing that hey... "it works"?

People will tell you how to make code review (peer-programming, formal inspections etc) what to look for in a code review, studies have been made to show the number of defects that can be discovered before the software hits production etc. But how do you convince programmers to accept a code review?

share|improve this question
48  
It's a strange question. Code reviews are a very important part of the job; a programmer who actively fights them is simply not a good programmer. I mean, what if people don't like spaces and newlines as well? how do you convince them that using whitespace and indenting code is a good idea?.. –  Pavel Minaev Aug 21 '09 at 7:37
2  
So you think I have a choice to work with good programmers only? Or you think, I should just not care? –  abc Aug 21 '09 at 9:18
2  
You don't have to work with good programmers. You just have to force them to become better programmers. Even the best programmers still need to improve their skills. It's part of the job description and good programmers will spend at least half a day learning new techniques. (During working hours, btw. Anything studied during private hours is just for personal improvements.) –  Wim ten Brink Aug 21 '09 at 11:26
8  
You have to "force them to become better programmers" ? Maybe... you want to be a good role model, and a steward who guides them to become a better programmer. Attempting to "force" people can often lead to the wrong result entirely. –  Leon Bambrick Aug 21 '09 at 14:16
2  
How do you "make" people do anything? Threats of violence work. On the other hand, if you would rather work with people and create a cooperative culture, I would advise that you not try to "make" anyone do anything. –  Rein Henrichs May 1 '11 at 15:17
show 4 more comments

migrated from stackoverflow.com May 1 '11 at 15:08

This question came from our site for professional and enthusiast programmers.

marked as duplicate by gnat, Kilian Foth, Dan Pichelman, Robert Harvey, Michael Kohne Nov 7 '13 at 1:33

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

34 Answers

Make the code review a mandatory part of committing code to the mainline repository.

This is what I have my team do and it increases the quality of the code and almost completely eliminates the possibility of questionable code making it into the code base without being specifically marked "technical debt".

We use Git and Gitorious to manage our code bases and repositories.

There is a mainline repository, nobody commits directly to this but pulls from it to their private clones on the Gitorious server.

The work and push commits to their private server side clone and then perform merge requests to apply for their changes to be incorporated into the mainline.

Someone else responds to the merge request, preferably the team/project technical lead, and reviews the code the changes, applies them, compiles and runs tests, and if everything is ok, pushes the changes to the mainline repository and closes the merge request.

Team/Project Technical leaders have their merge requests responded to my someone else on the team, which is a great learning experience for junior members if a peer isn't available to do it.

Either way now more than one person is on the hook for poor code being committed to the mainline repository and affecting the rest of the team and the company as a whole. If a more senior member of the team responds to the majority of the merge requests then they are more qualified to fix or reject poor implementations or designs before they can affect the entire code base.

These mini code reviews are much more useful than traditional code reviews which are usually a waste of time. Design reviews are much more valuable, because a proper* implementation of a **poor design is much worse than a poor implementation of a proper design.

share|improve this answer
add comment

I was astonished nobody mentioned yet the Agile practice called "The Perfection Game", wich is part of the Core Protocols. Core Protocols suggests a kind of code review easier to accept for programmers.

Summarily this is how it works:

  • the programmer ask to some potential reviewer if he is OK to play a perfection game
  • if reviewer is OK, the reviewer check the code an give a value of 1 to 10, depending on the value he believe can add to the code (for instance if he says 1 it may mean either this is a piece of shit but I do not understand a bit of it, won't be of any help, or it can mean this is nearly perfect, I couldn't do better. If reviewer says 10 on the other hand it would mean: your code is a piece of crap, but you have luck : I can help you with that because I'm an expert of this kind of crap).
  • the reviewer explains what he likes (not what he dislikes) in the reviewed code
  • the programmer then ask what would be needed to make that code perfect and the reviewer explains it.
  • the programmer then thanks the reviewer and change his code if necessary.

Of course this can be iterated until the programmer stops looking for perfection, or the reviewer can't help any more (he will give a low evaluation and the programmer will stop there).

This is a bit formal as it comes from Core Protocols, but the programmer is always responsible of the code changes and is asking review as a service. He usually won't use this kind of review to reject responsibility, on the contrary the programmer should ask to another reviewer if a given one won't be able to help.

Perfection Game may not applies to every case as it works on a voluntary basis (I understand the question as asking about mandatory code reviews). It also put to light that the label "code review" covers many underlying different practices (in the agile World only Perfection Game and Pair Programming are for instance two totally different kind of code reviews).

Some practice are easier to accept than others and it may also depends on the team and the individual programmer. As an afterword I would also answer to the original question by saying that code reviews are easier when there is trust between the members of a team.

Whichever the practices chosen, Trust toward fellow teamworkers is certainly at the core of code review acceptation by programmers..

share|improve this answer
add comment

That depends extremely on the programmer. I had loads of code reviews and some just ignore whatever you say - sometimes because they do not understand, but mostly because they simply ignore advice - especially when it's being pushed on them. It often looks like "they write bad code" (which they more often then less do - sigh) and they take that personally - even if it is not meant that way (positive ctiticism).

Interesting enough, the best way I found to get people to write cleaner code is to introduce automatic code inspection tools on check-in. So when their code doesn't meet certain standards (i.e. test-cases, comments, unsave code blocks), the commit is rejected. There are still some people who fall through the loops and write bogus, but it helped more than it hurt (in my experience).

share|improve this answer
2  
Automatic code inspection tools are there to be coded around. There is no insight gained. –  lutz Aug 21 '09 at 7:38
add comment

Make sure it's easy to discover new code. Preferably, notify developers when code changes - don't count on developers to look for changed code. In a reasonably sized team automatically sending SVN diffs on commit is a great way of achieving this. Together with the Colored diffs addon for Thunderbird code-reviews are a breeze. I read most of the code committed (we are six developers on my team). Not all developers will necessary read commits by doing this, but it will become really convenient for those who are motivated to do so.

share|improve this answer
add comment