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.

I am running a project where I pay developers to contribute to my semi open source project. My problem is that it is easy to hire developers, but it is very difficult to get anyone to do code reviewing. I have repeatedly attempted to get the senior developers to review the code of the junior developers. I have attempted to pay them more, give them higher status etc. But they just seem to want to code instead of reviewing code.

Now I am considering forcing all developers that want to code, to also review code, but I have a feeling that it is a bad idea.

My question is, therefore, how can I motivate capable people to review the code of others? How is this done in successful open source projects like Linux, PostgreSQL, LibreOffice etc.

share|improve this question

marked as duplicate by gnat, MichaelT, GlenH7, MSalters, Robert Harvey Apr 29 at 19:05

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.

8  
Could be your review tools are unpleasant to use. Could be the code to be reviewed is horrible. Could be that reviewing just isn't fun. –  david.pfx Apr 28 at 11:11
36  
The "it is easy to hire developers" idea is so pervasive, and yet so flawed, that it's ruining the industry. –  MetaFight Apr 28 at 11:30
2  
Why are you "guessing" it's not fun? Have you had a conversation with the developers? If you think cleaning a toilet is not fun, try cleaning one on a sinking ship. Everything is relative. –  JeffO Apr 28 at 12:27
1  
The @gnat dup-meister strikes again! And in that link I found an interesting tool that might be relevant: ostatic.com/blog/open-source-code-review-tools. –  david.pfx Apr 28 at 13:24
8  
Just sort of at face value: it's a business proposition -- you're paying them for their time and expertise. Since you're paying them, make clear up front that you're paying them for coding and review. –  Rob Y Apr 28 at 14:28

10 Answers 10

up vote 28 down vote accepted

Code review is a solution to a problem. Do you have a problem and will "Code Review" solve it? Are the other people checking in bad code? My guess is they are to some degree, but maybe your other coders don't think it is so bad that it is worth the time/effort to do a review. Ask your senior devs to come up with a solution to limit the amount of bad code checked into the system. They may come up with more effective solutions.

Solve the problem of having bad code (Code review is one of the pieces to the puzzle). One way to motivate developers is to stop the adding of new functionality until bugs/bad code are fixed. Most devs like building new stuff. Make the seniors fix bugs and refactor bad code. Maybe they'll learn it is easier to catch this in the code review.

Again, identify a problem and give your senior people a chance to come up with a solution.

You can't keep paying people who don't do what you tell them, so make sure those things are very important. It helps if everyone agrees there is a problem and contributes to the solution. Eventually, you have to make them accountable. They don't do code review, they take a lesser position or get fired.

In the future, hire people willing to do code review or at least let them know this is part of the job so they can make an informed choice.

Edit If you are having problems with your jr. dev's code, your seniors may feel the quality is so low, it would be faster to rewrite it than go through a review and correction process. It would be important to stress the long-term benefits of taking the time now to review, in order to give feedback to junior developers to make them better in the future.

share|improve this answer
13  
Code Review is a solution to the problem that developers do not spring forth from the head of Zeus with full knowledge of all languages. As a by-product, the process can help reduce the size of your open issue list, but Code Review should be present even if you have enslaved several minor deities and have zero open issues. –  Sam DeHaan Apr 28 at 19:13
9  
Code review is a solution to a problem. It's prevention, too, not just cure. –  starsplusplus Apr 29 at 9:40
1  
You can't say no one should ever check in code that has mistakes. That is just unrealistic. Code of junior developers should be reviewed by senior developers so that the senior developers can help pass on good habits and help to correct bad habits. –  rooby Apr 29 at 14:01
    
@Sam:Code review doesn't generally buy much if it is just code review for code review's sake. Unless fixing grammar/spelling errors in comments is a big gain. A senior developer reviewing the work they tasked to another is indispensable as the senior knows exactly what the code is supposed to do and how it fits in the system. Whereas, just having a random group of developers review code seldom provides much value as they won't have the necessary knowledge to know if the code is doing what it needs to do in the way it needs to be done. Testing should have already uncovered the obvious errors. –  Dunk Apr 29 at 17:20
    
@rooby - Sorry, I forget programmers take things too literally. I don't expect anything to ever be perfect. –  JeffO Apr 30 at 14:38

Perhaps one way around the clear reluctance of your existing developer base to review code would be to introduce a 'tutoring/ reviewing' position that operates separately.

In this way the individual taken on for the tutoring position would generate interest from individuals seeking to improve upon their abilities while also having a clearly defined role of reviewing the work of the junior developers.

This would leave your senior developers free to focus on their strengths - developing.

share|improve this answer
1  
Good suggestion, but I am unsure about how to recruit someone directly into reviewing without having them first produce some great code, which I can review? –  David Apr 28 at 11:17
    
@David - Just make them do a code review. Give them a chunk of code (It is open source after all.) and sit with them through a review process. That is the task at hand. There are other skills that are needed like reading, coding, understanding the project and your coding standards, but that will come out as you test them on a code review. –  JeffO Apr 28 at 11:25
    
You could go on the strength of the candidate's references - perhaps even ask for a sample of the individual's coding style (I am sure that your senior developers would pitch in their review here if it means that they get spared a lot of code review in future) The objective here would be the use of a dedicated tutor/ reviewer as a force-multiplier, both reviewing the code and actively improving the first-submission code quality of the junior developers. The tutoring aspect may sweeten the deal for some of those who feel that code review isn't their cup of tea. Also - what JeffO said. :) –  Avestron Apr 28 at 11:30
2  
a tutor is a bad idea - it sounds like the incumbent devs will just ignore them,and they'll quit when they get demoralised enough. Better is to get a build manager, lock the trunk and say "no code gets merged to trunk until after it has been reviewed". –  gbjbaanb Apr 28 at 16:23
    
While a specific position may sound like a good idea it is not likely to be sustainable and you will miss out on many of the good effects of reviews. One person doing it all will become fatigued. And possibly the best gain from reviews is cross training and knowledge sharing between developers. –  Beth - loud ninja - Whitezel Apr 29 at 5:00

it is easy to hire developers

This is the problem. The developers you hire simply are not motivated or don't have experience to keep the codebase in high quality. You should focus on hiring developers who take it upon themselves to keep the code quality high. And finding developers like that is extremely hard. Both because there are not many of them and that figuring out from interview that developer cares about code is tricky.

share|improve this answer
4  
I think there is a much larger segment of people who know how to code that will follow the best practices if put in the right environment with incentive, training, supervision and accountability. Even great programmers may just feel a need to work only on "their" section of the code base and not worry about everyone else. –  JeffO Apr 28 at 11:29
4  
@JeffO It is not about being great. It is about seeing value in quality code and taking it upon themselves to ensure the project has quality code. One of the major incentives is wanting the project to succeed. And quality code is one of the requirements for that. –  Euphoric Apr 28 at 11:33
11  
+1 It may be easy to hire developers, but it's still hard to hire developers who care. –  Eric King Apr 28 at 15:02

First answer the question why you want the developers to do code reviews. Are there too much bugs, are coding conventions not followed, is knowledge not shared, is code checked in with insufficient tests, etc.

Then discuss this with the developers. Tell them what you expect and ask them how they would improve on this. They should all agree to implement these changes or it will be difficult to enforce them.

If everyone agrees code review are useful, which they normally are, then a new rule might be that from now on only reviewed code can make into the product.

To be able to do a review it should be clear what is expected. So define what rules apply to code, like:

  • coding conventions (naming, code style)
  • default solutions
  • constructs to avoid

Use good tools, so make it easy to find code to review, see the changes and talk about those changes.

Code review should become part of the normal workflow to implement a change. That could be organized in this way:

  • a developer is expected to find someone else to do a review of the changes to include.
  • or, for every committed change you must do review.
share|improve this answer
2  
If all you get from your code reviews is the 3 bullets listed above then those code reviews are a complete waste of time. The reason for code reviews is to find BUGS and incorrect/unexpected behavior. The other stuff is just fluff, most of which can be handled automatically without a code review at all. –  Dunk Apr 28 at 16:41
1  
Another reason for code reviews, which you seem to ignore, is that the code quality improves even before the code is ever reviewer by a reviewer. And then there are "design reviews": In the simplest form, you figure out how to solve a problem, then you talk for five minutes to someone else, and surprise, surprise, you come up with a better solution. –  gnasher729 Apr 28 at 16:54
    
@Dunk, If code review is more to a find Bugs, then this is primarily a QA Task which would be done by QA Person/Team not by programmers. Peer testing is OK sometimes but it can not replace QA Testing. However other points like coding conventions are more developer oriented tasks, So a senior (but dedicated i think) developer could be deployed to do this. –  kuldeep.kamboj Apr 29 at 10:06
    
@kuldeep.kamboj: I don't think a 2 programmer development organization has a QA team. –  RemcoGerlich Apr 29 at 12:43
    
If it is two programmer development organization, Then obviously OP miss to inform/require programmer before hire. He must hire people with in advance requirement of Peer Testing Responsibility. So only programmer ready to do Peer Testing can be hired. –  kuldeep.kamboj Apr 29 at 13:04

In any plan there is liable to be some difference between what you would like to be high priority and what actually is high priority. Since you're paying, you pretty much get to decide what is high priority, and you pretty much get to tell the people you're paying to work on high priority stuff first.

So, if they're avoiding code review then either:

  • You are not making code review higher priority than writing more new code. You might think you are, when actually you're saying "can we get this done by the end of tomorrow?" and letting them respond "sure, if we drop everything else". "Everything else" means they skip code review on this and anything they've done recently that needs review.
  • They aren't doing what you asked them and paid them to do. Since you're prepared to pay extra for code review and they'd still prefer to write new code without review for less money, you have to find out from them why code review is so unpleasant.

Either way, to get the developers involved you could look into the Scrum concept of a "Definition of Done". You could have this even if you're not doing Scrum, and include code review in your team's definition. That is, they can't say that X is finished until X has been reviewed, which means they have to find someone to review their code (and the code of the juniors they're responsible for). Hopefully this will lead to them trading off reviews with each other.

Another way to present it is that by reviewing code they are familiarising themselves with parts of the system they may need to work on in future. If you encourage shared responsibility for the whole code base, as opposed to ownership of components, then you force more eyes onto each piece of code. Granted that's not the same as a formal review phase, but it's a move in the right direction since it achieves the same goal of avoiding using code that only one person thinks is correct.

Of course if they really hate reviewing then they could choose to do a completely cursory review, passing everything more-or-less without reading it. In that case you'll have to drill deeper into the problem and convince them to do a good job. To demonstrate that review is beneficial you can point to problems that occur due to someone having goofed off a review they were supposed to do. Professionals hopefully respond fairly strongly once their mistakes are seen to cause real problems, but often less strongly if they're failing to tick off what they perceive as a pointless task. If you cannot find problems that can be at least partially ascribed to lack of review, then consider again how you decided that review really is beneficial for this project.

Another possibility, that I can't rule out from what you say in the question, is that the coders you have simply aren't very good, or at least aren't very good at producing a working project without additional technical support. You don't say anything about testing, bug-fixing, or QA in general. If all they want to do is write code, and don't want to make it actually work reliably, then naturally not doing code review would be one part of this. The flip side, I suppose, is that maybe your coders are so universally awesome that their code always works first time and doesn't need testing, bug-fixing, or review. That seems unlikely!

share|improve this answer

Getting coders to do code review

This is a cultural problem, and you're currently in a state of equilibrium because they're used to not having to give or receive criticism of their work. To change the culture from the top-down, you need to lead, direct, and enforce, as necessary, to maintain it as a cultural habit.

I suggest you start off by scheduling the review, asking your best coders to walk you through their code, with the other offering feedback. If you think there's too much speculation or unproductive interaction, get it back on track. If small issues are identified that can be fixed on the spot, have it done on the spot. Take notes and ask questions that demonstrate you're doing your best to follow along. Keep track of issues that are emphatically known to need addressing, i.e. technical debt, and have them devote time to specifically addressing the technical debt issues.

You might begin with an entire architectural overview, and then on a weekly or even daily basis begin to dig in to each module they've written. I recommend starting with your best coders and then working your way through to your more junior developers so that they don't feel as threatened having seen others accept criticism, even though they might get quite a bit more criticism.

The only way you'll get change at this point is to take ownership of the problem and lead the change.

share|improve this answer

I am guessing you have already decided that doing code reviews is a necessary task if you are willing to pay senior developers more for it.

Most developers (in my jr developer experience) would rather develop something new, rather than look back over code that they, or somebody else, have already done. That being said, there are extra points that need to happen or it will make the task especially undesirable.

Coding Standards. There should be some kind of standards on how code is generally displayed so everyone isn't doing it differently, everywhere. Better coding standards, if followed, will mean quicker and less painful reviews. It also gives the code reviewers a reason to tell a developer why it needs to be redone without saying, "I don't like how you did that," which nobody likes to hear or say.

Code Reviews should have an impact. After a code review, the results should be acted on rather soon. There is no point to doing the review if nothing changes anyway. Likewise, the developer who created the code needs to know why their code had to change, or they may just keep doing the same thing in the future and the code reviewer will get tired of repeatedly fixing the same thing.

Keep it fair. Like any undesirable task, the same person will not want to do code review all the time. If somebody gets stuck doing it most of the time, everyone else will try to push it onto that person, either actively or just by not volunteering. You will need to set up "turns", or if you only have a a few people you trust to do the review, have everyone go through the code at the same time together. That will establish what your coding standards are and get the newer developers comfortable in doing the code reviews.

In conclusion, code review often seems like a pain to everyone involved. It should not be an assigned task to a single person / couple people. Only by sharing the pain, acting on the outcome, and getting the newer developers to learn from their mistakes can everybody come out ahead.

share|improve this answer

I used to work at a publishing, rather than software company, but the principle is the same.

In the publishing company, the road to management (and higher paying jobs) was editing/reviewing the work of other, more junior, professionals. A few people were content to remain senior writers, with no editing/reviewing responsibilities, but "most" (more than half) of the competent people wanted these review/editing jobs. NOT having one of those jobs was something of a stigma, because it was "career limiting," so most "juniors" tried to advance to editing as soon as possible.

share|improve this answer
    
The difference being that the "most" competent people in the sw world don't want the management jobs. They love the development side even if it was "career limiting". I guess it would be akin to someone who dreamed of being a writer throughout their lives and then choosing to be an editor who never gets to write because it is better for their career. I also really question your most competent people assessment as I would expect exactly the opposite even in the publishing world. Those who love their craft tend to be the best; the others strive for management. –  Dunk Apr 29 at 17:09
    
@Dunk: I would say that about "half" of the top third, and most of the middle third wanted editorial work. That gave management enough choices from the top, with the "difference" going to candidates from the middle third. I didn't say "most competent people," I said, "most of the competent people." –  Tom Au Apr 29 at 19:42

I used to work at a company that hired good, motivated developers and kept them. But we still felt there was value in code review: it helps spread the knowledge and just because one person feels that what they've written is good code doesn't mean there isn't room for improvement.

And we faced a similar problem. Coders preferred to code. To say nothing of wanting to shy away from the complex power dynamic of having a peer perform a task which is deliberately critical of your work, however well-intentioned.

The answer we found was: beer.

This is not a joke. Once every 2-3 weeks the company would pay to get some good beer in, and we'd spend the last 1-2 hours of the working week doing code review. The free beer was a good motivator, and the conviviality engendered by mild inebriation removed much of the sting from the process.

It worked so well that what had been a negative thing that all the programmers wanted to avoid became something of a monthly highlight: we all got free drinks, a pleasant atmosphere and got to improve our code all at the same time.

share|improve this answer
3  
Inspirational! Thank you for sharing! –  David Apr 29 at 11:54

I work for a big box retailer. Where old habits die hard. Like, really hard. So I thought I may share my experiences.

The problem we had is visibility. No ones code was really visible to each other. They were using CVS and we won the victory of converting everything to use git. This began to turn the tide. We implemented Gitlab (similar to github, basically all the same features hosted on your servers). We protected the master branch (production ready code). This created a snowball effect of sorts. If a developer had something that needed to be merged in to production or a testing environment branch, it would need 2 senior developers to look at the code, make comments, suggestions for improvements, ask questions etc and eventually give a thumbs up on the merge request. From there, it would be merged to the environment.

Stepping in to just rewrite the code you don't like isn't going to help you OR your developers. It's the definition of insanity.

Here is a summary of the ways we make sure we produce quality code.

  1. Unit Tests - Break down the requirements into small bite sized logic questions and write tests around what the application chunk is supposed to be doing, expects and returns.
  2. Write Documentation - Pretend you are a new hire and are asked how to do X. If your code has no documentation then basically any developer stepping in to the code is like "Whoah... " and usually takes a good bit of ramp up time just to figure out whats happening under the hood. I just don't believe in the mentality "Good code documents itself".
  3. Empower Developers - Don't take the keyboard. Ask thought provoking questions in an effort to bring forth thought driven decisions.
  4. Write More Documentation - I can't stress good documentation enough. It's literally invaluable to an organization because people change jobs, leave or get fired and sometimes all the information about a module goes with them.
  5. Using version control (Git + Gitlab) - This is such a huge help. If you are able to implement something like this I highly suggest you do so. This will get many more eyes on the codebase, merge requests, productivity etc from your team. It's amazing.
  6. Coding Style Guide - Create a style guide that you would like to see followed when others are coding. At the end of the day, all your code should look like it was written by one person. Not 100.
  7. Make it fun - What? Huh? Coding fun? If you can make it more like a game (think xbox achievements) then people tend to try a little harder and it feels a lot less like work. We use a Jenkins build server which compiles some of our client side code. During this process we used CSS Lint and JS Lint to find issues and bad habits in our codebase and document them. If you were the one that made mistakes in the code, you got dinged. If you cleaned up code and reduced issues, you got points. We noticed immediately that developers were scouring the codebase in search of issues that would earn them points. (The Continuous Integration Game plugin)

I hope this adds some value to your situation and this post helps you in some way. So many great responses in here.

share|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.