Skip to content

Instantly share code, notes, and snippets.

@bitwiseman
Last active September 14, 2018 20:55
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save bitwiseman/f7ad10f7883f858da657c2f63ebac943 to your computer and use it in GitHub Desktop.
Save bitwiseman/f7ad10f7883f858da657c2f63ebac943 to your computer and use it in GitHub Desktop.
Better code reviews and PRs: Commenting less, doing more and better

(Note taker arrived late)

Frustrations/Issues:

  1. Commenting when making edits would be more effective - "Be the change you want to see in the code"

  2. Over-broad notification - Stash adds you to everything you ever touched for notifications

  3. Typo and formatting nitpicks - "This whitespace is off."

  4. Non-functional comments - personal or other poorly defined stylistic preferences/suggestion

  5. Friction during code review process

  6. Siloing - reviewers not being familiar with other areas or being unfriendly from reviews outside their prefered colaborators

  7. Lack of trust (due to trolling poor tone in comments or due to over-investment in created implementation)

  8. Too informal or overly formalized process

  9. Lack of clarity about the purpose or design of the change (spec and other planning docs) - High WTFs per minute

Ideas and Potential solutions:

  1. rubber ducky

  2. Ask for early feedback from one person

  3. Get early feedback on spec before implementation to avoid "calling their baby ugly"

  4. Having people who write spec not be allowed to write code

  5. Link to JIRA

  6. Add spec in commit message

  7. Talk in person or via video

  8. Automated linting and fixing, code style enforcement (MD lint)

  9. Comment severity classification: NITPICK, BLOCKER, (general)

  10. Office hours (video or in-person) for PR discussion

  11. Github PR template

  12. (Wish there was) Github comment template/suggestion

  13. Meta-review that reviews CR for how the process went

  14. Small teams make things easier

  15. (Stash) Notify groups of users but pseudo user that points to a DL user.

  16. (Github) ".codeowners" file controls who is requested to review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment