Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?

Implementing a Strong Code-Review Culture

As an author

  • Provide more/sufficient context around changes
  • Linking to the issue isn't always enough
  • Challenge: 2 paragraphs of context

As a reviewer

  • Offer compliments in PRs when you learn or something is done well
  • Ask questions rather than make demands/commands, engage in conversation
  • Commands and demands create 3 actions: just do it, argue, or ignore
  • Commands don’t recognize author’s thought process
  • “What do you think about ?”, “Did you consider...?”, “Can you clarify...?”
  • Soften suggestions
  • Questions and engagement mean LEARNING
  • Don’t say “Why didn’t you just...?”
  • “just”, “simply”, “easily” don’t give good feels. Don’t use them when asking questions!
  • Be positive.
  • Socratic method to get more critical thinking, ideas

Conflict

  • Conflict can drive higher standard of coding
  • Disagreements might just be a discussion of tradeoffs, might be “not the way I would have done it”
  • Committing to master? Go review the commit anyway and discuss the problem of not having a pull request with the person
  • Enlist your team to help on process

What to Review

  • SRP + SOLID principles
  • Naming (naming can help lead to better discussion and responsibility)
  • Complexity
  • Test Coverage (Not QA!)
  • Personal checklist (what your passions are: security, performance, duplication, etc.)

Style review?

  • Style is important
  • Giving feedback about style puts the you in a negative eye of the author
  • Write it down, automate it

Benefits

  • Better code
  • Better people! Better developers
  • Better ownership (less “my code” “your code”, since we all reviewed it together)
  • Better versatility (everyone knows the system)
  • Healthy debate

Other notes

  • A lot of parallels to pairing too!
  • Refactoring doesn’t change tests often, so base your questions around why we’re doing the refactor and what we gain from it
  • Asynchview helps changes stand on their own
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.