Code Review Best Practices

We know that Code Reviews are a Good Thing. We probably have our own personal lists of things we look for in the code we review, while also fearing what others might say about our code. How do we ensure that code reviews are actually benefiting the team, and the application? How do we decide who does the reviews? What does "done" look like?

In this webinar recording, Trisha identifies some best practices to follow. She talks about what's really important in a code review and sets out some guidelines to follow in order to maximize the value of the code review and minimize the pain.

0:00 Introduction: Reading other people's code is hard
5:21 What to look for in a code review
6:37 Different code review workflows
8:42 One Size Does Not Fit All
9:01 Code Review anti-patterns
18:49 Why perform code reviews?
23:07 When do you review code?
24:37 Who is involved in the review?
26:39 Audience questions
31:36 Where do you perform the review?
36:53 What do you look for in a code review?
42:01 How do you perform the review?
51:26 Summary
53:40 Audience Questions

About the Presenter
Trisha Gee has developed Java applications for a range of industries, including finance, manufacturing, software and non-profit, for companies of all sizes. She has expertise in Java high performance systems, is passionate about enabling developer productivity, and dabbles with Open Source development. Trisha is a Java Champion and is a Developer Advocate for Upsource and IntelliJ IDEA.

[joshs]

Very clearly explained, human-centered, and thought-provoking. Thank you Trisha

[burhanuddinrashid]

Thank you @Trisha Gee for explaining this in detail.
I like the Why, When, Where, What and How parts.Its very easy to remember.

[TheNaddaeus]

The lowdown is at:
51:26 - Have Clear Objectives

Thanks Trisha!

[jimigrok]

Thank you for sharing your knowledge - nice talk!

[eduardozaldivar]

Hi Trisha Gee, Thank you for sharing your book! Great examples.

[DarkNivo]

Awesome stuff. Helped me a great deal. Thanks

[donnyjoe]

Thanks a lot Trisha. This was really exceptionally helpful for me!

[sandeeptiwari_yt]

Anti pattern are real bottleneck for code review process at most organisations. Well Explained 👍

[Luis-xxsh]

Thanks! those are very useful & well explained dos and dont's

[Mohamed-ufjh]

Thanks for sharing your knowlodge and book

[Leon-Li]

I dont like those reviewers dont point out which part of the code needs to be changed and why they want it to be changed. It doesnt help me to improve at all.

[馬莎琳]

"who write this code " it happen to me when i read my code 6 months later, and complaint a lot, why I write it that way.... 🤣🤣

[peternagy]

Hi Trisha Gee,

thanks for the talk just watched, this is the best talk about this topic so far I encountered and I agree with most of the points that you mentioned, but I have few questions/debate, I hope it's not too late to ask.

You did not separate open source and in-house source code projects review process, do you believe they have the same code review requirement by nature? Like gateway type is necessary for open source projects to protect their code base, but in-house application team misusing/misunderstanding gateway type code reviews.

You mentioned you like when juniors do the review, but they might be pressured to accept the change. And in my opinion, the reviewer needs to have a good understanding of the code/language/tool/framework to be able to review. For example, it's useless to review UI code by backend developers who do not understand the code/language/tool/framework used. So what is the point to ask juniors or someone who does not understand the code to do the review?

Why do you believe if something merged to trunk it's already released/in production? It's like code review a git-flow specialty.

I do believe one of the most important code reviews is the security review, that you didn't mention, and because most of the developers are not aware of all vulnerability, it's usually another team (security) needs to do together with the developer team.

The review process is different when you have an existing code base or the development has just started. If it's open source project one commit needs to focus on one particular change, but in-house projects or the early stage of the development the commit doesn't have to be this focused. Inside one commit you might refactor something which is more efficient to develop a particular feature and segregating to different commits it's impossible. Do you agree?

You mentioned the author needs to commit in small pieces to be able to be reviewed. Do you agree sometimes that makes even harder to understand the changes? and also makes even harder to implement?

I totally agree with you, XP is the best practice to do code review, but do you believe team members who work together for ages still need the same review attention than someone who just joined into the team?
What do you think team members can achieve maturity/trust level when they don't require review?

Do you believe review, misunderstanding the review can be harmful to organizations?

Thanks for your time, I'm really looking forward to your answers.

[zes]

wrr, no such thing as havex or criticx or not, do, say infix any nmw and any s perfx

[sairajkalkundre]

The words "Generally Speaking" should be automated in this video as it was spoken 18times.

[papanino]

If you hate code reviews so much why do a whole presentationon them? The speaker seems to be someone who takes critism of her code very personally so I'm not sure why she's even giving this talk. Besides, if the goal of a code review is to just approve code then why not skip the review altogether and push directly to production? Overall this was a terrilble presentation that only shows the speaker's hostility and bad attitde.

[joachimdietl]

code reviews are a waste of precious time