@Michelle_Philbrick Thanks for giving this option some thoughts! It’s definitely helpful to know what we would need to prepare well, if we were to implement this. This is probably a topic on which @itsjeyd will likely have better ideas than me since he’s closer to it, but giving it a shot:
- I have hundreds of PRs that I comb through every few weeks and it’s very time consuming. Making sure every two weeks that maintainers have responded to specific ones might be a bit of a challenge to keep track of, especially since we’re aiming to get edx-platform as a maintained repo. There’s so many OSPRs in that repo.
That does seem like a lot of PRs to go through! I wonder how much of the hundreds of PRs you have to comb through are recurring/long to merge, vs new ones? How many new PRs do you get each time, vs old ones that you have to keep going back to because they don’t get merged or closed? Speeding th would decrease the number of PRs that you have to go back to.
We might need to find a good way to quickly identify which PRs would qualify for review by any developer - ideally this wouldn’t require any manual combing, but could be the contents of a specific board. What if it were simply “any PR which isn’t either rejected/closed or merged after X days out of the draft stage?” Then a filter can display them on a board, and core contributors can comb through those directly? And you then don’t necessarily have to go on all the PRs to remind the maintainers - the escalation is simply a tool added to your toolbox when trying to help PRs get reviewed, if some languish and you think it’s useful, you can escalate them yourself to specific core contributors.
For the systematic part, maybe a bot could help? Eg. X days after the PR goes out of draft, a bot posts an automated message on the PR comments, saying that it can now be reviewed and approved or refused by any code core contributor. The bot could do the merge / close of the PR based on a list of core contributors.
- For some repos, like edx-platform, it would be good to nail down exactly which CCs have rights to review/merge in what specific areas. For instance, all CCs might be able to review/merge PRs on edx-platform, but what if only a few CCs have expertise in the area you’re looking for? Can we make this list more explicit about who can handle what?
That would be helpful to figure out in any case – it’s important to maximize the quality of the reviews, and making it clear for each part of the code who is responsible for it is a good way to ensure that. There are also bots that automatically use this kind of information to ping the right people for the review.
The information would also be a good way to identify where we don’t have that expertise available, either temporarily, or at all. These are probably areas where we should probably escalate it immediately to other core contributors – who might not be familiar with this part of the codebase, but if contributions are blocked because nobody with the expertise is available to review them, that problem perpetuates itself. Doing reviews and merging code in is a good way to build expertise - we know we can trust core contributors to care about the code quality and project’s best practices.
- Also, 2U has a basic breakdown of which 2U teams own certain projects on edx-platform, but it’s not as specific as we need it to be. It’s also not always clear what project a certain PR is related to.
I also know that @jmakowski is also building a list of relationships between features ↔ repositories, which might maybe be able to help here?
- There may be certain PRs that 2U would want to review that they don’t get to within that 2-week period. How do we flag these?
2 weeks is already a long time for getting a PR review, in an open source project. For Open edX that timeframe would be a great improvement compared to the current situation – but that’s part of the problem we have as a project, and that we need to solve. Often large open source projects try to provide reviews within a day or two - because this is when contributors are most likely to keep working on it, and have more latitude to change their code. If we are not able to provide a review within 2 weeks, maybe it means that the changes to that code base are not a priority, and then it’s fair to let someone else take the decisions, since they are doing the work?
- We’d need to figure out a cadence for getting stalled PRs to the CCs. Maybe every few weeks Tim and I could post stalled ones in the CC slack channel and have people self-assign?
Advertising them this way regularly would likely be a nice way to get more people’s attention on them. Especially while working on catching up with the reviews backlog?