Process/Patch Review

Patch Review

Patch review is an important part of collaboration. It can help increase code readability, and maintainability in the long run. Done well, it sparks joy in our community and our daily jobs.

Follow those simple steps to get an harmonious collaboration in the code review platform:

  • The differential revision should be what goes to Git.
    • Patch description should match the commit message.
    • For small non-commit-message addendum use a separator (---).
    • If more detailed explanation is needed, use design tasks, and/or Wiki.
  • Be pragmatic and assume good faith.
    • Sometimes there are only minor changes left to be addressed by the author. In those cases it is fine to "green-light" while explicitly asking those changes to happen.
    • When clarifying comments in the code or nailing down a name, consider moving the discussion to a more efficient medium.
  • Green-Light means it is good to go.
    • If the patch is not “green-light” the author is expected to give the patch another iteration.
    • If a reviewer is only reviewing part of the patch (e.g., UI), approve the patch (or not) and set other developers as Blocker reviewers - otherwise the patch will show as accepted.
  • Design, then code.
    • If the patch needs agreement on the design task first, it should be put on hold (status: Close - it can be re-opened later).
  • Developers usually reply within 3 working days.
    • Otherwise reach out or remove them from the reviewer list.