...
Community contributed pull requests are also known as “OSPR”s (suggestions for other names welcome!). OSPR is a shorthand term for all pull requests issued against the `openedx` GitHub organization that are not generated from the tCRIL Axim or 2U teams. These PRs need an amount of management:
...
What follows is a set of questions and answers to situations that an OSPR Project Manager may face. There will certainly be situations that arise that are not answered here. Questions about these should go to the tCRIL Axim team.
Status Alerts / Prompt Descriptions
...
If PR has been open for a long time (>30 days) and there’s been no action on the pull request, AND the pull request is waiting on the author NOT a reviewer, @ the author and ask if it should stay open.
After two weeks with no additional response, close the PR with a kind message (e.g., “Thank you for your contribution! We’re closing this PR at this time because it is stale; you may reopen this pull request, or open a new one, when you have time to come back to this work.”)
If a pull request has a 2U author and a 2U repo owner, OK for it to stay open unless it’s been open for a long time (~30 days), in which case, OK to check on status. You will be able to distinguish a PR designated as an “OSPR” because it will have the
open-source-contributionlabel on it:
PRs generated by 2U and tCRIL Axim engineers do not have this label.
...
For contributors on a new repo (even if they’ve previously committed to a different repo) their PR needs to have approval before checks are run (since the tests execute code).
If an OSPR Project Manager has a technical background (to the extent that they’re able to judge whether a PR is introducing malicious code), they may check code changes themselves and let repository maintainers and/or tCRIL Axim engineers know whether a PR is safe to run tests on (or not).
Failed checks are authors' responsibility except for needing to get a green CLA check.
If author has signed a CLA but the CLA check doesn’t complete and shows as “pending” for more than a few minutes, it is likely stuck (it does not mean the check failed). A rebase should fix the problem. If not, pull in Feanil/Michelle.
If a PR is still in draft state, there is no need to ping authors about failing checks.
If a PR is not in draft state and the author asks for review, it usually makes sense to ask them to fix failing checks first.
However, if a PR introduces a large, impactful set of changes that requires input from more than one team (e.g. product and engineering), it can be a good idea to get the review process started early, regardless of failing checks. We want to avoid scenarios where authors spend days fixing small issues and getting the build green, only to be told by product and/or engineering that their changes are not going to be accepted or have already been introduced by another PR.
...
Alert them that they need to follow the procedure for new employees. Their manager (or 2U IT) needs to go through the process of creating a tCRIL Axim ticket to register them in the system.
Their team must know what this means; it is not your job to walk them through a process people in their org know!
...
Yes, but only someone with write access can perform the merge. You may need to remind the reviewer who gives the approval that they will need to perform the merge.
Note that this step is time sensitive. If a PR sits around too long before merging (for weeks or, in the case of repos with lots of activity, even just a few days) it may need to be rebased again, increasing effort for both the author and – sometimes at least – their reviewers.
Useful Links and Resources
External Contributions GitHub Board
https://github.com/orgs/openedx/projects/19/views/1
PRs not authored by 2U or tCRIL Axim engineers go to this board when they come in.
Once added to the board, they need to get reviewed and merged.
Use the board’s columns to change the status of a PR (you can also change it on the right side of the PR page itself). The following guidelines should help determine appropriate status (as well as labels to add to the PR to capture additional information): Contributions Board Status & Label Breakdown.
...
CCs on a repo can provide review and formal approval for a PR and merge it.
Many repos do not have CCs or have so few that they may not be able to handle all incoming OSPRs. In this case, you would need to get 2U (or possibly tCRILAxim) reviewers to approve and merge PRs.
Contributions Board Status and Labels for Open Source Contributions
| View file | ||
|---|---|---|
|
Backstage
https://backstage.openedx.org/catalog?filters%5Bkind%5D=component&filters%5Buser%5D=all
...
Current OSPR Project Managers
Michelle Philbrick (Axim,
@mphilbrick211on GitHub)Tim Krones (OpenCraft,
@itsjeydon GitHub)
Open TODO items
Update the bot to provide a welcome message saying test won’t run if you are a first time contributor
Come up with a list of common issues when running checks - OSPR Project Managers can link to this doc when people have questions
Sarina & Feanil will help create
First part done - message about how to get CLA help is now messaged when the bot makes a comment on a PR without a CLA; see pull request for bot changes.
Draft document up for the “doc for questions”. Not sure where to put it in the docs site, but comments can go here: https://docs.google.com/document/d/129cyHROlSXJRpt8O8TuqQTr6uVi-C7aY_6NgopFnq5k/edit?usp=sharing until I’m able to get a PR up in the proper place.
...