Goals of code reivew are exchanging knowledges, increasing code awareness of people, and finding potential security issues or breaking changes to users. Stylistic issues should be written down somewhere else, automate it, delegate to Robots.
As an author, share QA steps to let people try at local if they want. Share what solutions considered and provide alternative solutions.
As a reviewer, ask questions, don’t demand people what to do. Provide actionable feedbacks. Suggest working alternative solutions. Generously compliment on things you found good.
Keep in mind code review comments are impersonal, temporary, specific. It’s about the code.
Only block (aka request changes) the Pull Request / Merge Request when things will go terribly wrong. Heavily influenced by @orta while contributing to danger/danger. He is a great reviewer. Give you motivations to keep going.
I spent a lot of time in the past to suggest things I think is good, but people come at different experiences. So now I get over myself and if I really do care, fix it post-merged.
- Keep it small
- Small is easier to review
- Ship multiple PRs behind feature flag
Do not use requested changes. An external contributor who does not have merge power sees a red blocked changes that is just discouraging.
Do not suggest removed code without reasons. Your project dynamics for external contributor who just wants to fix your thing is discouraging.
- thoughtbot/guides code-review
- Confessions of a programmer: I hate code review - Made of Bugs
- How to Do Code Reviews Like a Human (Part One) - mtlynch.io
- How to Do Code Reviews Like a Human (Part Two) - mtlynch.io
- Building an Inclusive Code Review Culture
- Modern Code Review: A Case Study at Google
- Pull Request from Artsy
- A guide to mindful communication in Code Reviews