Code Reviewer Guidelines - internetarchive/openlibrary GitHub Wiki

Pull Requests

We use assignee to denote PR ownership. If you are the assignee, then you should have the PR on your todo list until you merge or close it.

  • Assign yourself to a PR if you have the time to take on the responsibilities of ownership (described below).
  • Don't assign others to a PR. Feel free to ask someone to take ownership, but respect others' time restrictions.
  • Avoid assignee=author. In the case where the PR author is also a maintainer, we will strive to have another maintainer own and merge the PR to ensure these steps are followed fairly by all.

The assignee of a PR is responsible for:

  • being the primary contact for the PR author. Be polite; you're the face of the community to this contributor.
  • managing the PR's labels. Add Needs: Author Input or Needs: Review as necessary.
  • ensuring the PR doesn't get stuck. Avoid leaving the author wondering about the state of the PR. If you don't have time right now, saying "I'm a little swamped now but will try to get to this in _" is better than radio silence for a week.
  • getting the PR code reviewed either by yourself (often so) or by someone else.
  • getting merge approval. If a PR requires a special deploy, label as Needs: Deploy Approval and get that approval before merging.
  • testing the PR before merging. Comment about how you tested in the PR. If any changes are made to the PR code, you will have to test it again before merging.
  • merging (or closing) the PR.

Each Monday (as of 2022) we triage PRs (excluding drafts) and make sure they have leads assigneed so that nothing gets stuck.