Code review anxiety
Carol Lee (Clinical Scientist) shares her research on code review anxiety. We dive deep into her recent research paper “Understanding and Effectively Mitigating Code Review Anxiety”. We get into all the nooks and crannies of this topic — common code review myths, strategies for coping, the need for awareness and self-reflection, the value of exposure and practice to build confidence, the importance of team dynamics, respect, empathy, and connection, and more. This show is jam-packed with goodies for everyone…and we even give a nod to the work we did on our podcast Brain Science.
Discussion
Sign in or Join to comment or subscribe
LosManos
2024-07-10T17:57:46Z ago
We have a thriving pull request culture in my present project. We have, and have had, both newbies and scarred developers. We have no anxiety around PRs. Our rules are:
Then I add “If you read my PR, I count on you doing the same for me.”
With that we have set the rule that a failing PR is not a fail; it is part of the process.
The PR is a forum for us to discuss code. For that we tend to prefix the comments with PRAISE, SUGGESTION, EXPLAIN, CHECK and FIX. With the prefixes we set the tone and the reason behind the comment and can be terse and exact without having to deal with too much psychology.
=> So the PR did not fail. It is just part of the process, like having bugs and fixing them.
Here are examples of how the prefixes work:
“PRAISE: Good refactoring, I like how you remove the negation.”
“FIX: A null check is missing.”
“SUGGESTION: Split line.”
“CHECK: Is it the right enum used here?”
“EXPLAIN: Is this comparison really correct? or seems to overflow to me.”
Then in real life we get laxer as we get to know each others’ strengths and weaknesses.
Not laxer with code quality but with the rigidity. Knowing the colleague does what is supposed to be done we can ok the PR relying on the comments being considered.
For example: Today, yes today, while reading an extra sensitive piece of code (well written btw) I ended up with calling my colleague and we discussed the code for 20 minutes. We put some “CHECK:do-x” in proper places. An hour later the PR was updated. There was still some corners so we spoke (yes - it was sensitive code) and we put a “FIX:do-y” in there. But I approved of the PR knowing he would adapt the code to both of our liking. One hour later it was running in production.
We have used this for quite a while and it is slowly spreading. I did a write up here: https://www.selfelected.com/code-reviewing-comments/