Getting coders to do code review

Question

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.

Answer 1

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.

Answer 2

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.

Answer 3

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.

Answer 4

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.

Answer 5

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!

Answer 6

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.

Answer 7

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.

Answer 8

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.

Answer 9

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.

Answer 10

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.