Nonviolent Code Review
This is an essay. It expresses the opinions and ideas of some mediawiki.org users, but may not have wide support. Feel free to update this page as needed, or use the discussion page to propose major changes. |
A collection of thoughts on nonviolent code review (NVCR). As the title suggests, this is an attempt to apply the ideas of Nonviolent Communication (NVC) to code reviews.
Note that this is not about the application of NVC during code review. NVC is about emotional reactions and the needs of individuals, which does not apply to code review (unless things get into a meta discussion about how someone feels about code review). NVCR is an attempt to re-apply the approach of NVC to engineering principles and the requirements of technical systems. NVC is for discussing the feelings and behavior of people, NVCR is intended to be for the discussion of code and other technical systems. NVCR is intended to keep technical discussions focused and productive, just like NVC is designed to keep personal communication focused and productive.
Four elements of feedback
[edit]- Observation: what do I think is happening here?
- Concern: what bad things may happen if this is done?
- Principle: on which principles is this concern based? Which principles does the code violate, and in what way?
- Request: how can the concern be addressed? How can the desired change be made without violating agreed-upon principles?
Of course, not all reviews can or should be so elaborate as to follow all the suggestions in this document. But the more complex and contentious the code review gets, and the more emotional the discussion becomes, the more useful these patterns may prove.
Three kinds of conflicts
[edit]- Misunderstandings or lack of information. This can be subtle. Ask carefully to establish a shared understanding. Avoid assumptions.
- Difference in value or judgment. Work together to formulate a specific trade-off. Then find the appropriate person to make the call on the trade-off. Typically, that person would be a (technical) product owner.
- Ego bullshit. This includes "not invented here", "we have always done it that way", "I spent a lot of time on this", "I'm not stupid", etc. This happens in all of us. Spot it, forgive yourself and others; smile, shake your head, give your inner child a lollipop, and move on.
Golden rules
[edit]- If something seems silly, ask why it is done that way before saying it's silly. Someone probably failed to see an important aspect - maybe it was you.
- Make sure answers given during code review make it into documentation.
- No stonewalling: when criticizing, always provide a clear way forward.
- The person writing code should have autonomy over how the code is written. This autonomy is limited only by agreed-upon principles.
- Matters of preference can be discussed, but should not block a patch.
- Be clear about what you consider a blocker, as opposed to a matter of preference, or a request for clarification.
- Follow MOS:WTW. In particular, avoid judgmental terminology.
- Back off when frustrated or angry. Take some time to calm down, then start asking questions.
- For meta-discussions (providing feedback on how someone does code review), use to the tools of NVC.
Further Reading
[edit]- Guidelines for a healthy code review culture
- Compassionate Coding by Aniket Kadam.
- Code Health: Respectful Reviews == Useful Reviews from Google's Coe Health series
- Unlearning toxic behaviors in a code review culture by Sandya Sankarram
- The Ten Commandments of Egoless Programming on Coding Horror
- Conventional Comments
- Nonviolent Code Review by Valtteri Laine
- Egoless Engineering by Dan McKinley