Skip to content

Instantly share code, notes, and snippets.

@eliperkins

eliperkins/implementing-a-strong-code-review-culture.md

Created September 4, 2015 20:40
Show Gist options

  • Select an option

    Learn more about clone URLs
  • Save eliperkins/a3b75d44f934be2f1942 to your computer and use it in GitHub Desktop.

Select an option

Learn more about clone URLs
Save eliperkins/a3b75d44f934be2f1942 to your computer and use it in GitHub Desktop.
Download ZIP

Revisions

  1. eliperkins created this gist Sep 4, 2015.
    49 changes: 49 additions & 0 deletions implementing-a-strong-code-review-culture.md
    View file
    Original file line number Diff line number Diff line change
    @@ -0,0 +1,49 @@
    # 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 <command as a question>?”, “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