Adapted and reworked from a talk originally given at RailsConf 2017.
The intersection between humans and technology has never been simple or easy....
For further actions, you may consider blocking this person and/or reporting abuse
Just a quick "Thank you" for that article, as it probably was one of the best articles on peer reviews I've ever read.
Thanks for your kind words! I'm very glad you got something out of it 😃
Exhaustive coverage of the tools, goals and people dynamics. Excellent coverage (no pun intended) and has helped me to articulate a few concrete dimensions that we can work on to take our mechanical code review process to a valuable progress accretive culture.
With progress being the key goal, we have also linked our appraisal to the review scores.
Thanks!
We all know we should and we all know why...
Thanks for the constructive tips on how to do it better :)
If someone who read this wants to review my code, that would be awesome !
Really liked the article, will do some reading on your resources. Thank you
One idea we use at work, that I really like is that the reviewers are asked to put "minor", "major" or "explain" to each comment, so the reviewed person can easily distinguish concerns from suggestions and questions.
Nice post, Vaidehi. I think it would be interesting to combine your results with those from Stack Overflow's yearly developer survey, which cut its responses along language and age lines for various analyses.
In particular, I wonder if your rankings of how programmers of different languages perceive the benefits of code reviews could be explained by the typical profile of someone using those languages. For example -- are those using Swift, a more recent language, more likely to be younger (humbler?) devs who are receptive to code reviews?
Maybe it's because my office deals with safety critical code, but we do pretty thorough code reviews. 5 - 7 people are invited to the reviews, and given a minimum of 3 days to submit comments; we have our own customized tool for managing all of this. The 1 main thing I found that helped to make the reviews more effective was to limit the size of the code being reviewed: no more than 400 pines of code. Peer reviews are difficult, and draining on the reviewers. Thus, we need to keep them short, so the reviewers can maintain a solid focus on the entirety of the material being reviewed.
I think it also helps that doing peer reviews is ingrained in our office culture. Everyone does them, both as author and as reviewer. Thus, everyone knows that issues found during a code review are a good thing, and not an indictment on the programmer.
I really liked this post. I'm going to comb through it again when I get back to the office, and see what we can do to make our peer reviews even better.
Enjoyed attending this talk live at RailsConf.
I think it might, indeed, be time for a rigorous look at how code reviews are done in the web development world in the days of GitHub pull requests and test-driven development. (You are inspecting the tests, too, right?
)
The article failed to mention the one glaring reason why code reviews fail, i.e. CODING STANDARDS. Don't even start a code review process if your team does not refer to a central coding standard. Agree to a coding standard in terms of coding style and good/bad practices and make that as the bible when doing code reviews. This will weed out any biased opinions and your team will move fluidly.
Another thing, bugs don't care about your feelings. Sorry but that's the truth. Code reviews are there to improve code quality (less bugs) and definitely not to hurt your feelings. So put those feelings in a box. Code reviews should not be emotional. It's just code, man.
Your team will know if you're a newbie programmer but most people would not care. We are all just humans. People will be more concerned on how well you're learning and whether you're not making the same mistakes over and over again. This is so that you can be a valuable member of the team. So you can either fight the process or just accept the criticisms (good/bad) and just see all this as a learning opportunity.
This was not a constructive approach to responding to this post in accordance our code of conduct.
This tone is unprofessional and language not inclusive. It's perfectly fine to disagree, but do so with respect. Think of this as a warning, but if it is not a workable for you, there are plenty of online communities without a code of conduct.
Hey there,
Did you get a chance to read the entire article? It does, in fact, highlight the fact that syntactic discussions that devolve into back and forth in comments can be resolved by adopting a coding standard on your team. Using a linter, for example, is a simple way to make sure that the whole team agrees on a bonding style.
If you read the anonymous responses from the survey—which are not written by me, but rather were contributed by developers around the world—you'll see that the "process" that you encourage people not to fight is clearly broken. Code reviews aren't always effective on a team, so it's up to us to evaluate what's not working about them and try to fix it. This article is an attempt to do that.
The thesis of this article isn't that bugs care about your feelings. Rather, the crux of the issue is that humans write code, and humans are more productive when they are seen and heard on a team. The attitude of "put your feelings in a box, bugs don't care about your feelings" is pretty dismissive to a huge subset of the engineering community and doesn't foster any conversation or change.
That's not how teams work. Someone's "feelings" are never "in a box." Sure, I've heard (colloquially) of people who are offended by any form of feedback, and that's dysfunctional on their part. But almost everyone is willing to accept feedback if it's delivered respectfully, and will respond with avoidance or aggression if it's not. So considering people's feelings in code review makes it a more effective and positive process for everyone. In fact, it's a professional requirement. You can't stop being kind and respectful to a person just because their bug doesn't care.
Bugs may not care about your feelings, but bugs are almost never the biggest problem on a software team. And "It's just code" is a short-sighted description of software development; it's almost entirely about human beings and the way they think and interact. The code itself is a triviality.
Reading through your comment again, I think you may have said some things you didn't really mean. I hope I haven't misjudged you and I'd be happy to discuss this further if you'd like to clarify.
Amazing writeup, thank you for collecting, analyzing and presenting the poll results. Code reviews introduced me to the social and peer to peer learning component of being a software developer which is one of the best feats of being one. Thanks again! 👌
Awesome article!
I read an article previously about improving code reviews here: tech.forwardfinancing.com/culture/...
It's interesting you point out the items that can be improved during a review, such as a PR template, including screenshots or just making your commits smaller. These are things I've been practicing for years and sharing with the teams I've worked with.
Personally, it always came down to empathy for me. And just like the article I posted above, it's about asking yourself questions that focus on the person asking for the review. And I think it's a two-way street, the coder should ask specific questions that help the reviewer focus on what they should review and the reviewer should understand the audience and context in order to provide a thorough review - if that makes any sense.
I've started doing self-code reviews, where I basically critique my own code before assigning the PR. This critique will contain comments on the line I need help with or just posing questions that I find will help the reviewer help me out.
Anyways, thanks for the article - I love having data points for this :)
Wonderful post and thanks for sharing. One cultural aspect to code reviews I have always tried to instill in teams I have managed is the opportunity to learn from each other. It has always starts there for me. If people feel they can learn from a two-way interaction and are open to that learning, there is a greater chance for, to use your words, an energetic and substantive engagement.
It's also great to see that some of the resources I remember using early in my career have stood the test of time ;-)
Lots of really good pointers here! I can't wait to see the full talk when it comes out on Confreaks.
FWIW, Kent Beck did some number crunching on Facebook's code reviews and found that to minimize the amount of time it takes for code to be reviewed, the ideal number of reviewers to request is 1. (This is in a context where 1 review is needed but you can request from multiple people that they be that 1 reviewer.) Once you request multiple reviewers, chances are there will be diffusion of responsibility and no one will get to it for a while. If it's on the plate of someone specific, though, they feel a responsibility to get it done.
Otherwise, I can't see much to argue with here, and there's a LOT of stuff. Thanks for taking the time to do the research, crunch the numbers on the results, and share with all of us.
Great post!
Another method we use across projects is to perform automated code review (using codefactor.io) with every commit and pull request. It weeds out most of the "best practices" cases and tremendously helps to concentrate on a more meaningful conversation during Human code-review phase.
I would like to see code reviews more together with pair programming, especially if it's piece small enough you can rewrite together.
Interesting insights - thanks for writing this up.
wow, amazing post!
I've had serious problems with Code Review at a couple of places, and did a write up on how people tend to do it wrong: medium.com/@istumbler/code-review-...
Thanks for sharing!
great article Vaidehi, just a note that I'm getting these weird glyphs throughout the article, and on both Chrome and Firefox. 🙏
dev-to-uploads.s3.amazonaws.com/i/...
Bookmarked!
Really insightful, thanks for taking the time to write this up.
Really great post 👏👏👏