How do you make people accept code review?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.
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?
ACCEPTED]
I've found that people who don't like code reviews will do their best to work around whatever you put in place.
The best way to make sure that the code you work with is code reviewed properly is to work somewhere that treats that as the normal way of coding, and that only hires developers who are likely to fit into that environment well.
If you can't change who you work with, you might have success if you first give your own code for review. Encourage them to find fault with it (dare I suggest adding in a few deliberate mistakes?) so that you can demonstrate that it's not meant to be a way of criticising the developer in question. That's the main problem, IMO: people take code reviews too personally.
Well you can't really, if someone just flat-out refuses and you're not their boss.
But the main reason to adopt another style is ...
Consistency
I think, whatever crazy style there is, it must be consistent. So if a project is build in this style, it must typically stay in this style, unless there is an exceptionally compelling reason to change. So if their style doesn't match; it's a fact that it should be made to match. No question here.
There are other problems that can be noted that are just flat-out wrong. Security, general 'wrong-ness', etc. These are unquestionable and should be changed.
If someone refuses to acknowledge something as bad design, after you have shown them the correct way, and shown them all the risks and problems with what they do, I think you then need to decide for yourself how to act. But you would hope you are not working with people who are so unreasonable.
Simple: assign the important projects to programmers who do accept code reviews. That is a clear message: this project is too important to leave to amateurs.
The programmers who won't cooperate can do maintenance work. Your company will have plenty of that, if code reviews weren't widely done. Make it clear that this is a dead-end street. These products will decrease in importance, be dropped, or be replaced by newer products that were code reviewed.
Make it clear when hiring that code reviews are the norm in your company. Assign the new hires that do accept code reviews to the new projects.
Overall, this will tell your developers that the company is adapting code reviews. They can choose: go along or get out. There is no future in any company for people fighting the company.
I am programmer and I enjoy code reviews. The key to good code reviews in my experience is making code better.
When I do a code review with a colleague and he/she points out potential improvements or lurking bugs, I am happy. The code improves and we probably both get a little smarter. Why would I not like that?
Unfortunately some people see code reviews as a way to stress the fact that your code must have 4 space indentations instead of 3 space indentation or some other non-vital detail.
Make sure people who do code reviews can and will give useful information. If the only input code reviews provide is something which can easily be accomplished by using a tool, then code reviews are a waste of time.
Make sure code reviews are not a waste of time, and most developers will enjoy them.
The only reason I bought into the code review process was that a year or so ago the person that tried to impose them pulled the superiority card out and made us review each other's code for a given project. By the end of that project I could see the enormous benefit and I now try to instate code reviews on my projects.
"It works" is not enough. Code is not only written for machines to execute but also for people to read, adapt, and improve. If people can't find their way through code, they can't do all these tasks that are a mayor part of programming.
If Joe Programmer says: "Don't look at my code, I'm the only person who needs to understand it" I'd doubt this. Only in rare cases is code every touched by only one person. And even if this is the case: does Joe understand his own code next week, next month, next yeare? Probably not.
When I'm managing a project, if I say we are going to do code reviews, we are going to do code reviews. I will of course supply good reasons for doing them, but at the end of the day it is my responsibility and decision to implement them.
And if the programmers don't like it, they can find alternative employment. Most failing projects can usefully be improved by getting rid of half the developers anyway, and in my experience, it's the poorest developers that object to code reviews the most.
I've made good experiences with pair reviews. Two people are examining each other's source code while both are sitting in front of the same monitor. We make sure that it's not always the same people and that it's clear and agreed upon in advance what they both are looking for.
But still - doing code reviews requires open minded people who are willing to accept criticism.
I don't like code review when I am a developer.
But I really want to code review if I am a leader. We know why.
However, some developers will be sure to don't like code review, just like me.
How about tring anonymous code review?
We can write example code pieces or tips after code review to figure out what is better way.
A simple example:
original piece:
s=objGotFromSomewhere.doSomething()
better piece:
if(objGotFromSomewhere!=null){
s=objGotFromSomewhere.doSomething()
}
memo:
In some project , we got ... issue, caused by .. reason
tips:
always check ...
We can put this into wiki or somewhere , then talk about it in the weekly team meeting. Tech Leader will do it, and encourage senior developers and every team member to do it too.
I think, anonymous is important, that means no one knows who generated the bad code.
People may don't like code review, but will like to know what is better way.
In almost every commercial establishment (and most non-commercial ones, come to that) programmers do not own the code they write, their employer does.
In almost every commercial establishment (and most non-commercial ones, come to that) most programmers seem to think they do own the code they write.
This causes an awful lot of problems with acceptance of code review as a design improvement technique...
Ways to reduce resistance might include:
I'm not recommending any (well, maybe the first) of the above specifically.
What matters is to get a real understanding of what the real objections to reviews are, not what you're hearing on the surface. Maybe apply 5 Whys [2] to get deeper?
[1] http://en.wikipedia.org/wiki/Pair%5FprogrammingHaving a higher authority instate a review process should work. Even those who rebel against the idea at first should hopefully come around to see its benefits. If they don't... well, can't really help that.
If you're just a lone fighter among disinterested peers you'll have a much harder time. The key in both scenarios though is to make the reviewees understand that it's not a personal attack on their skills, but rather a team effort to squash everyones bugs more efficiently.
The best way is for the decision to come from the programmers that make it.
I would suggest to use some people skills and start with a discussion on how to make sure that the group can work together, I would also talk about ways to make sure that the workplace skills of the programmers stay up to date.
One way forward is of course to do code reviews, but there is also other avenues that might be successful, for example reading and discussing a book about how to program. I recommend "Clean Code: A Handbook of Agile Software Craftsmanship" ( http://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882 [1] ).
Another thing is letting each programmer present her/his projects in front of the group when they have finished doing something. That will naturally make sure that the ugliest things don't get written, and everyone gets to feel proud that they have accomplished something.
[1] http://rads.stackoverflow.com/amzn/click/0132350882Personally, I think a programmer who dislikes a code review is just a very bad programmer. If you write good code, you have nothing to fear from a review. You might actually be praised about your style. But if you make a habit of writing bad code then I can imagine why someone would dislike a review. But having someone else point out the errors in your style is just useful to improve your own experience! You should learn from a review, not shy away from it. It's an enormous boost to code quality.
When you do code reviews, make clear that you're doing this to improve code quality, not to punish the bad developers. Make sure your programmers will consider these reviews as educational, not as a way to measure their performance.
I've had plenty of code reviews and I've specialized in complex algorithms thus my code tends to look a bit complex for others. Fellow programmers do consider me to be very good at what I do and just love to review my code, because of it's educational value. And of course, occasionally they do find some small errors in my code but in general these code reviews are improving the quality of work of the person who reviews the code!
Don't always assume the problem is at their end. Maybe you're doing it wrong.
Find out why they don't like it and instead of automatically assuming that they are a 'bad programmer' firstly consider if they may have a point and that you may be a bad reviewer of code, or simply communicating badly. It's possible.
If your code review / style rules are not subjective, arbitrary or insane, then it will be easy to explain the rationale for them to any competent, professional programmer. If you can't, then maybe you need to rethink either what you are doing or how you are presenting it.
Otherwise, if the problem is with just one or two individuals, it is likely more of a deeper HR problem with those people and nothing to do with code review specifically, the code review thing is just a symptom of that bigger problem.
I never had a chance to be involved in code review, but I think it is surely an excellent practice.
For code review to be well accepted, you could have some training organised on how to make "positive criticism".
And, by the way, such a training could also be an opportunity to remind people the company/project development principles and do some team building.
We use ReviewBoard [1] to do reviews. At our company, reviews are voluntary: if you're doing anything of interest (substantial additions or changes) we expect you to create a review-request to be done by the collegues in your team. The selected team members can then approve the review or request further changes.
If you have a need to force reviews because a programmer isn't playing along, one might require all commits to the SCM to come with a review ID that is checked to be marked as approved before it is accepted.
[1] http://www.review-board.org/If you are in charge of the project from the start or are the lead developer, you should be able to conduct code reviews by instituting them as part of your project methodology. Make sure the reviews are performed by leading them yourself, if necessary. If you are in charge and some people won't follow your lead, then you need to either persuade them to get with the program or replace them with people who will.
If you are not in charge, then you have to persuade the team members that it is a benefit to them to perform code reviews. Start by persuading the other team members to review your code. Document your design and interfaces and walk them through your code. Then write up a post review summary and action items for yourself and distribute it to your team members. If there is a benefit from this, it will become quickly apparent to the team.
If there is resistance, you need to find out what people's objections are. They might be valid. For example, the project might be small enough and the entire team might be experienced enough that everyone on the project effectively peer reviews all the code in the project even without formal reviews.
I find code reviews most useful on very large projects where it is not possible for one person to really grok all the code, where there are inexperienced developers who need the feedback in order to develop their coding and analytical skills or where there are non-coding members of the team who must understand the design and the code. I find reviews essential or even mandatory when there are legal consequences or worse if the code fails (e.g. aircraft flight control systems). I find them redundant when working with seasoned programmers, where everyone on the team produces code, where the project is small and the team members read and critique the code all the time anyway.
The 1-3-2 principle applies here (I speak 1st person, then 3rd person, then 2nd -- in that order).
First priority is to practice what I preach, meaning I am first concerned to say "I" need code reviews (and doing them).
Then only if the atmosphere isn't self-adapting to code reviews, I start to say "people" need code reviews (and list the good reasons).
Only after that, if all else fails (and I'm in authority :) ), I lay down the law and say "you" need code reviews. This is the part that people have correctly enumerated on, but I twitch only when the 3rd person mentality is not last. If I fail to speak 1rst person fist and don't practice what I preach, not only am I an "ineffective" persuader, but I'm just that much more a jerk.
ps. Notice how I said all of that in first person :)
To offer up another perspective, I have sometimes discovered that code reviews drift into things that do not make sense to even discuss. Like I had this guy reviewing my code saying that I don't have the correct punctuation (a stop) in one of the comments I added to the code. While it is subjective whether such a thing is important (if you are writing inline documentation that will get pulled into released docs for example, it might be important), in my case it was clearly not.
I think code reviews are mandatory, I have almost always been helped by code review comments, and it sometimes takes a bit of time for a new team member to understand why they are important. But it is also important that objectivity is maintained, every suggestion should have an objective reason, and should not be made because of subjective preferences of a reviewer.
So to answer the question - it is good to mentor new team members on the importance and purpose of code review, at a stage where they are likely to listen. And with experience, they will learn that it is not personal. Also it is important to have the right people be reviewers, not people who impose their subjective opinions on others.
Review with questions, not commands
I think a fundamental problem with your review process is your idea that you need to force anything down their throats. Instead, ask questions like "Why did you decide to go with X approach instead of Y?"
This will make them think about their own coding process, instead of forcing them to accept your coding process. And hey, you might learn something too.
I think the biggest problem with code reviews are that some people can't do them right. For example putting final in front of class/method/parameter/anything doesn't matter and should not be in a code review. Another example is using the wrong loop since only foreach is ok. Require to comment everything. String +/StringBuilder/StringBuffer optimizations. If I got a code review with any of those things I would think that guy is an idiot and if I was not forced to change the code I would not.
Also sometimes code review tools are just wrong, for example default PMD [1] (Java) likes to say that class has too many methods, so what I did is to create abstract parent class and push some methods there. I could have split the class into 2 classes, but I think the API will be easier to use with all those method in one class (it's a small lib and my design decision). Some people will always use just the default, so it should not be crappy.
[1] http://en.wikipedia.org/wiki/PMD%5F%28software%29In my experience, pair programming has far more benefits than just reviews. Not only does it find more bugs, but it encourages more creative solutions, knowledge sharing, team building, open communication, and allows a team to continuously refine a natural programming style.
Every time, I've been on a team that tried reviews, it gets confrontational, and then fizzles out when a tough deadline looms.
It can be hard for a seasoned developer to adjust to the idea of reviews but when it clicks the momentum really picks up. Try it for a couple weeks and see if it works for you.
Try to sell them on the learning aspect. Almost every programmer I've met enjoys learning, and I personally have learned something at every code review (whether I was the reviewer or reviewee).
Once you start the reviews, however, it's your job to ensure they are worthwhile. Don't get bogged down in petty formatting arguments (take those offline). Use static analysis tools (like FindBugs [1] for Java), search for TODOs, review every compiler warning, check documentation for completeness, etc.
The more valuable insight found in the review, the more successful the reviews will be.
[1] http://findbugs.sourceforge.net/Code reviews are an essential part of preventing a major disaster early on. A lot of the major development shops REQUIRE code reviews on their projects.
To make it easier on people to accept there should be guidelines in place for the reviewer to follow. If the individual feels like they're being singled out or that the reviewer could have a grudge they may be reluctant to go through a review. If ahead of time everyone is informed of the guidelines and what the reviewer will be looking for, you may have better reception. No one likes subjectivity when it reflects on their career or performance.
Speaking from experience, I've worked with a number of developers on high profile government contracts who were anti-code review. Where did that get us? Way behind schedule and way over budget. Developers with something to hide are going to be resistant to people going through their work as they're well aware of their short comings.
It's been my experience that those who aren't willing or are resistant to reviews are also the same people who aren't willing to learn and adapt their style of thinking to the problem at hand which can be a slippery slope for a project when the person is in a decision making role.
Something else to think about; if the budget and company can make it available have someone who is tasked solely with code reviews or even bring someone over from another project to do the review. If there is no prior relationship, it may be easier on both parties.
If all else fails and you're concerned the persons attitude may be creating problems for the project overall, document it, and escalate the issue to a superior in a respectful manner. Going after someone or pointing out their short comings is going to look bad for you and may draw attention to your own work.
First off, as with all problems, start solving this one yourself. Make sure you are doing everything to make code reviews as productive and non-confrontational as possible.
I have a different situation. Since I'm not a very experienced developer, I would like to do code review for my code before every release. But most of the time, code review just doesn't fit into our tight schedule. Or, my company doesn't think it's important enough to make the case. Same for test-driven development. If I really have questions, I'll grab a senior developer to look at my code. Not ideal but doing so does help me improving my coding skill.
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 [1] 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.
[1] http://code.google.com/p/colorediffs/If they won't do code reviews, then simple... just lay them off. Clearly they aren't too concerned with improving anyway. There's no need to keep dead weight around.
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).
You need to accept that you can't "make" people accept code review.
You can make a compiler do what you want, but with humans, the best you can aim for is:
"How can I be more influential when reviewing another person's code?"
...and that's another question entirely.
How do you make people accept your code review?
You can't, so you don't. And you're not responsible for anyone else's work until you're asked to take responsibility for it.
You could approach your 'review' so it's more like 'feedback.' It's not you being the boss looking for things someone else did wrong, it's two professionals discussing different approaches to the same problem.
IMO, any two programmers will come up with different ways to solve a problem, and they can both be right.
Ask the reviewee, 'What do you see as the advantage of taking this approach?' Also provide the disadvantages you see and the advantages of your alternative approach. Ask for acknowledgement of the DA's you've pointed out- 'do you agree this could be a problem with the code you have?'
I think most programming conflicts come from value differences and guessing what might happen in the future. People can spend hours arguing over something completely indeterminate. Focus on real problems now, less on (but not avoiding) things which could happen later.
One more option: We hold periodic "reflections" on our process with all the programmers on a project. Part of this practice is to do a "root cause analysis" on the bugs that have surfaced recently. What caused this? Was this a regression? Was this a lack of IE6 testing? Would asking Joe about this helped? The goal being to figure out why we are having those bugs and what we can change to prevent them.
If code reviews are an appropriate tool, this will come out during the discussion. And it will be the programmers themselves who introduce it, and so you will get much more buy-in from the beginning. We agreed that all code needs to be reviewed before it's checked in. Developers generally ask for it, but if they forget, they sheepishly admit it... they know they aren't just defying "management", but going against the team's agreement.
(Or if it's not an appropriate tool, people get a chance to explain why.)
