Code Review
Last updated
Was this helpful?
Last updated
Was this helpful?
Describe what are your objectives when you review peer’s code ?
How do you do it ?
Share in pair
Don't review anything that we can automate
Spell check
Code beautifier
Small batches
No more than 200 to 400 lines of code (LOC) at a time
Brain cannot process and detect errors if too much data
At least two developers sitting together
Junior Dev should pair with a more experienced one
Whole team when the feature is risky or complex
Everyone code can be reviewed
Any Dev can review
The code of the most senior is also reviewed
The reviewer
should be :
Humble
Give feedback
Don’t blame
Need to get knowledge of the functional analysis
Checkout the code on his/her workstation and run it
Could be on-demand
Or, if the CI quality gate not passed
And anyway, before a story can be considered “Done”
Better code
Ensure code meets standards
Find bugs
Ensure code does what it’s supposed to
Check code is understandable
Share Knowledge
Spread code Ownership
No better way to learn the right practices and promote the company’s code guidelines and requirements
Collaborate on Design
Improve communication inside the team
Every team members talk to each others
Foster companionship and improve team cohesion
Motivate to progress
If you keep in mind that your code will be reviewed by some colleagues, you will put more care into it
You will take the time to properly :
Name identifiers
Write tests
Introduce well-fitted abstractions
Use carefully the enterprise wide patterns
A developer :
Picks up or is assigned a task
Implements the code to the best of his/her ability
Submits it for review
The reviewer checks the code and, if everything is OK : the code is merged into the main development branch.
Learning at the center of the process
The purpose is not to check if the code is correct
But to share with other developers what changes have been made
A collaborative and iterative approach to
Check the code frequently while it’s being built
So design feedback comes at the right time
My job as a Reviewer is to find problem
When you try to find problems, you will always find more problems
Always remember that you bring you own preferences / biases
Nit picking
Spacing is wrong / Formatting
Automate this kind of check
Waste of time for a human
Design changes when the code works
You find it too late
Design must be discussed early on
Inconsistent feedback
Feedback biased because I have followed a security training
Next week it will be on compliance for example…
To avoid this :
Have clear guidelines
Use checklists
The Ghost Reviewer
No time for it
The bigger the code review, the more likely it will receive a green check
Ping Pong Reviews (Between reviewer and author)
Happens when you goal is to find problems
Have a Definition of Done
Clear guidelines
Form groups of 3
Each team member take :
A role card : keep it secret
A checklist
A code sample
5’ in solo :
Prepare a code review as a code-venger : role-play based on your role card.
Review time :
Each member explains his/her discovery in the code to the two others.
They have to guess who is the code-venger
And keep note how they feel during the review
Who was who in your group ?
How did you feel during feedback session ?
Judgmental questions :
Avoid to say : Why didn’t you just do ___ here?
Try it : What do you think about ___, which has the benefit of____?
Being sarcastic
Avoid to say : Did you even test this code before you checked it in
Try it : This breaks when ___. Can you please address this case?
Passing off opinion as fact
Avoid to say : This component should be ___.
Try it : Since this component ___, it could be made ___. This will improve___
Prepare once again the review of the code by being yourself
Try to avoid behavior sentences explained before
Bootstrap your team check-list
What / When to review ?
“Being the most senior person on the team does not imply that your code does not need review...”
