View Source

This is out of date

Below is a set of guidelines for Core Contributor - Committers on how to review OSPRs.

Champion Coordination:
- Reach out to your edX Champion to agree on the process. You will want to align on answers to questions such as:
  - Are there any types of OSPRs that your champion wants you to always review or always skip?
  - When and how OSPRs should be merged, e.g. during specific hours or weekdays?
    - For ideas on how to coordinate merging, please expand the section below.

Repos that don’t automatically deploy: no restrictions on merging.
- These are repos that are not setup for CD or require updating the version in another repo for the changes to deploy.
- There is no need to coordinate the timing of merging into these repos.
- But inform the champion to deploy the changes in the dependent (higher-level) repo, when ready.
Repos that automatically deploy: coordinate merge times.
- For example, here’s an agreement between a Committer and their champion:
  - Committer pings the champion to do the actual ceremony of hitting the merge button, which ideally happens mid-day Eastern time so that both are around in case anything comes up, production-wise.
  - Reasons the champion may hold off on hitting the button are: release timing issues, a production CAT-1 is in progress that needs addressing first, etc.

Selection and Prioritization:
- You can find all open OSPRs on this JIRA board.
- We encourage you to review OSPRs of other authors and companies, not just your own.
- edX has identified OSPRs that would benefit from review by a Core Contributor Committer and labeled them with cc-review-please.
  - The ”CC review please” filter on the Jira board displays the OSPRs that are still available for review by a Committer.
  - If you choose one of these OSPRs, assign it to yourself and add the label cc-reviewer to it.
    - Note: You may not be able to update the Jira tickets yourself right now. We are working with edX IT to address this access issue. In the meantime, contact Natalia Berdnikov (Deactivated) to update the ticket for you.
- If there is no available or appropriate OSPR in the ”CC review please” filter, then you may search for another one on the board as follows.
  - Look for tickets that are not assigned to someone else in Jira by using the “Unassigned with CLA” filter.
  - You can pick the oldest (brown and red in this JIRA board), or what is most relevant to current projects, or use another appropriate strategy.
  - If you have a desire to adopt a ticket assigned to someone else in Jira, you may reach out to the assignee and check if they are ok with you reviewing for them.
- Review-only (not Merge):
  - If you see a pending OSPR for a repository that you do not have merge access, you are welcome to add your review on that PR. That may help expedite the review process.
  - Once your review is complete, either:
    - Option A: tag Natalia Berdnikov (Deactivated) on the PR and she’ll find someone else to merge.
    - Option B: tag @core-contributor-committers in slack and see if another Committer is able to merge for you.
- Do not merge “Community Manager Review”
  - If you are reviewing an OSPR that has a yellow “Community Manager Review” label in Github (same Jira status in the ticket), please do not merge. This label indicates that author has not signed the Contributor Agreement with tCRIL, and edX cannot accept their code yet. Once author signs and legal team processes it, Natalia will confirm so by posting in the PR and changing Github label and Jira status. Only after that the PR can be merged.
Labeling:
- Please add cc-reviewer label in Github to PRs you want to review, and tag Natalia Berdnikov (Deactivated).
Product:
- All PRs that are end-user-facing or introduce a new feature or change functionality, need to be first reviewed by the edX Product Owner of that feature.
- You can tag Marco Morales (Deactivated) on the PR to route the Product review request.
- Exception: if PR is fixing an obvious visual issue like a typo or distorted layout, Product doesn’t need to review.
Quality:
- We have high expectations of Committers' ability to review with high code quality in mind, including architecture/design principles and definitions of done (i18n, scalability, etc).
- If you have any doubts about accessibility, please tag Jeff Witt (Deactivated) on the PR.
Support:
- If you need anything, please tag Natalia Berdnikov (Deactivated).
- If you have any questions/feedback/input on this process, please comment on this wiki or reach out to Natalia Berdnikov (Deactivated) or Ned Batchelder (Deactivated) .
Contacts: