Conducting Humane Code Reviews
@AdrienneTacke
👋🏼 DevFest Baton Rouge 2022
✋🏼✋✋🏿
Who has performed a code review?
✋🏼✋✋🏿
Who loves doing a code review?
Why?
There are so many good things about code reviews!
// Benefits are synonymous with our goals
Catch design flaws
Ideal Code Review Goals
Ensure code readability
Ideal Code Review Goals
Validate necessity
Ideal Code Review Goals
Confirm functionality
Ideal Code Review Goals
Top Three Code Review Complaints
Subjectivity
Tone of Voice
Process Loopholes
(All of "human-origin" 😅)
...What we can do better?
Before you do ANYTHING, gain consensus with your team!
Develop a Team Working Agreement and enforce it!
What's a Team Working Agreement?
Team Working Agreement*
What are our goals for code reviews?
How long do we give reviewers to approve pull requests?
Do we require a minimum of two approvers for pull requests? Three?
What happens when we need to deploy an emergency fix?
What code conventions do we follow as a team?
*(This can take multiple conversations and will evolve over time!)
Top Three Code Review Complaints
Subjectivity
Tone of Voice
Process Loopholes
(All of "human-origin" 😅)
Subjectivity
Tone of Voice
Process Loopholes
What's subjective?
From a pull request for the materialize repo
From a pull request for the handsontable repo
"You should really indent lines 23 and 27."
"Replace var with const! C'mon, it's 2023!"
"I'd rather use regexes here."
"TotalNumUsers is better than TotalUsers."
...What we can do better?
🤖
Let the robots take over!
(they're better at it anyway)
Prettier and ESLint have saved team relationships!
Objectivity Always Wins
What's Objective?
"This change should probably go into our utils library. It will be re-used quite a bit!"
"I've run this on several browsers and this toast actually pops up on the top-right but our ticket specifies bottom-right! Can we fix it so it matches our requirements?"
"I'm having trouble keeping up with the logic on lines 457-464. Is there a way we can simplify it or make it clearer?"
"This method is being deprecated in two weeks! I'd suggest replacing it with the authenticateUser method from our authentication library!"
🥰
If you see something you like,
say so!
Subjectivity
Tone of Voice
Process Loopholes
Objectivity
Subjectivity
Tone of Voice
Process Loopholes
Objectivity
From a pull request for the materialize repo
💩
"This implementation is terrible."
"What a stupid way to do this."
"Why would you do this? Only juniors write something like this!"
"Are you really using lodash, really?"
"This implementation is terrible."
"What a stupid way to do this."
"Why would you do this? Only juniors write something like this!"
"Are you really using lodash, really?"
...What we can do better?
TL;DR Don't be a jerk!
Suggest with facts
Reject with courtesy
Clarify with an open mind
Subjectivity
Tone of Voice
Process Loopholes
Objectivity
Empathy always
Subjectivity
Tone of Voice
Process Loopholes
Objectivity
Empathy always
No Reviews(!)
Bias
M.V.E.
Minimum V.E.
Minimum Viable E.
Minimum Viable Effort
...What we can do better?
Establish code review policies
Auto add developers to PRs
Use CODEOWNERS file
in GitHub
Enforce the process for EVERYONE
Subjectivity
Tone of Voice
Process Loopholes
Objectivity
Empathy always
Subjectivity
Tone of Voice
Process Loopholes
Objectivity
Empathy always
consistency
Because even after the PRs are merged, you still have to work with your teammates...
💛 this talk?
Let everyone know!
@adriennetacke
Thank you DevFest Baton Rouge!