Code Review

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.

References