openedx/wg-coordination

Facilitate monitoring and guiding of OSPRs

Opened this issue · 22 comments

antoviaque commented

This issue is to follow up on a topic from the contributor meetup working group

See https://discuss.openedx.org/t/pull-requests-review-delays/10497 for the context

  • Monitoring PRs is 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

  • Facilitate quickly identify which PRs would qualify for review by any developer - ideally this wouldn’t require any manual combing, a filter can display them on a board

  • Link repos to specific people:

    • 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
    • 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

  • Provide better information about the current situation on the different repos, since that can be different

    • => to link to the effort to document who the maintainers are?
    • Ned: We have a CODEOWERS, but it’s incomplete, and GitHub points out that it has errors
      • Xavier: Do you know if the file currently maintained? And how does that relate (if it does) with the process to identify maintainers in 0001 Use Backstage to Support Maintainers — Open edX Proposals 1.0 documentation ?
    • review bot could use a list of core contributors for the code
    • Jenna is also building a list of relationships between features ↔ repositories
itsjeyd commented

Closely related: #104

itsjeyd commented

Challenge

  • Monitoring PRs is 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

Comments and additional details

  • Ideally, repos that are part of the maintainership program would not need OSPR managers to help guide the PR review process.
  • At the time of writing, OSPR managers are responsible for helping with coordination for all OSPRs, irrespective of whether their repos are part of the maintainership program or not.
  • Until a decision is made to limit OSPR management to unmaintained repos, OSPR managers would benefit from being able to quickly identify OSPRs that do not get picked up for review by the maintaining team within a certain time frame (e.g. two weeks).
    • Rather than checking OSPRs for updates round-robin style (or based on the date of the Last PM check), OSPR managers could prioritize following up on PRs that have been waiting for maintainer review for two weeks or more.
      • For repos maintained by 2U teams, the agreed-upon approach that OSPR managers are currently following is to periodically provide Ned and Kelly with a list of stuck PRs, and for the two of them to follow up with maintaining 2U team(s) to get engineering review going.

Potential solutions

💡 To some degree, the Contributions board already supports this use case: It is possible to filter project boards by when items were last updated using last-updated:NUMBERdays syntax.

For example, filtering the Contributions board by last-updated:14days will list all OSPRs that haven't been updated for 14 days or more.

Updated can mean any of the following:

  • Created
  • Reopened
  • Edited
  • Commented
  • Labeled
  • Assignees are updated
  • Milestones are updated
  • Transfered to another repository

So it is not possible to filter out only those OSPRs that have had a response from a maintainer within the last two weeks.

However, when the number of OSPRs to manage is large, being able to get a list of PRs that haven't seen any relevant updates within the last two weeks, and prioritizing those for immediate follow-up, might still be useful for the purpose of reducing lead time for PR reviews.

(Not just in terms of identifying OSPRs that are stuck waiting for engineering review, but also when it comes to spotting OSPRs that should be closed because they have become stale due to inactivity of their authors.)

Changes applied

So far, two additional views have been added to the Contributions board:

  • Stuck waiting for review, showing OSPRs with a status of Ready for Review or Scheduled for Eng Review that haven't been updated within the last 14 days.
  • Stuck waiting on author, showing OSPRs with a status of Waiting on Author that haven't been updated within the last 14 days.
  • 👍1
itsjeyd commented

FYI @mphilbrick211 @e0d ⬆️

  • 👍1
e0d commented

Oh nice, that's a very useful query and useful views, thanks for doing this.

  • 👍1
mphilbrick211 commented

@itsjeyd @e0d I have a concern with the 14-day window of the views... I do a full tracker sweep every 2 weeks (and there are hundreds of PRs), and there may be a few where it's on me to change the status, but I haven't gotten there yet... so it would be shown in that "stuck for 14 days" when in actuality it isn't.

e.g. If something is in "Waiting on Author" or "Ready to Review", sometimes it gets reviewed or even merged before I get back to it (even if it's just a few days), or someone has started reviewing it, but I don't change the status to "In Eng Review" until I check back in on that PR.

I check in on PRs daily, and update the ones that have actions come through, but if I don't get notified on a PR, it might be a week or two before I look at it again, and that would have nothing to do a with reviewing team being slow. Just saying that it might not be a good reflection of actual status.

Should I plan to do a full sweep once a week instead? It would be a lot, but if we're trying to push things through faster it seems like that's the way to go?

itsjeyd commented

@mphilbrick211 Hm... I might be missing something, but with the scenario that you're describing I don't think PRs would be shown in the "stuck for 14 days" views: If a PR gets reviewed (= commented on), that should bump the last updated field to a date less than 14 days ago. So it should disappear from the "stuck for 14 days" views. Are there any specific examples where that didn't happen?

mphilbrick211 commented

@mphilbrick211 Hm... I might be missing something, but with the scenario that you're describing I don't think PRs would be shown in the "stuck for 14 days" views: If a PR gets reviewed (= commented on), that should bump the last updated field to a date less than 14 days ago. So it should disappear from the "stuck for 14 days" views. Are there any specific examples where that didn't happen?

This is a good point - I didn't remember we had that field that would auto-update.

  • 👍1
itsjeyd commented

🚧 WIP 🚧

Challenges

  • Link repos to specific people:

    • 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
    • 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

  • Provide better information about the current situation on the different repos, since that can be different

    • => to link to the effort to document who the maintainers are?
    • Ned: We have a CODEOWNERS, but it’s incomplete, and GitHub points out that it has errors
      • Xavier: Do you know if the file currently maintained? And how does that relate (if it does) with the process to identify maintainers in 0001 Use Backstage to Support Maintainers — Open edX Proposals 1.0 documentation ?
    • review bot could use a list of core contributors for the code
    • Jenna is also building a list of relationships between features ↔ repositories

Comments and additional details

Ref:

... currently it is a bit tricky to tell who to ping on some parts of the code. Maybe having to specify this to a level that a bot is able to ping people on its own would help simplify things, too?

Ref:

Right now, information about who to ping is spread out over several resources -- a few public wiki pages about the CC and maintainership programs, Open edX Backstage, and a spreadsheet that is available to Michelle and me but not the wider community. For maintainership specifically, information from the wiki and Open edX Backstage overlaps to a large degree but last I checked the wiki seemed to be a bit ahead of Backstage.

If we could unify the different resources somehow and find a way to make the information that they provide more discoverable (e.g. by having the bot surface it on PRs like you're suggesting), that would be great -- it would not only help Michelle and me in the context of OSPR management, but also empower PR authors to get in touch with reviewers themselves.

Ref:

If anything, I think we need to make sure you both have less work to do, by automating things like pinging the right reviewers. Imho it would be a better use of your time to allow you to focus on things like escalating PRs that don’t get attention, or monitoring the time it takes to merge PRs to proactively suggest improvements – ie things that actually require human attention and inventiveness.

--

Essentially, we want to:

  • refine information about code ownership
  • improve discoverability of information about code ownership

We also want to continue assigning code ownership (via the maintainership program). Fortunately, that's already happening and we can consider it out of scope here.

Ultimately, we would like to enable the https://github.com/openedx-webhooks GitHub bot to utilize information about code ownership so that we can automate certain steps of the OSPR management process (ref):

... for example having the bot suggesting/assigning/pinging reviewers based on the code that’s being modified

So we need to:

... find a way to unify these resources, and write a proposed specification for the bot to automate [surfacing relevant information on OSPRs]

However, we will need to take into account that:

[Michelle and Tim] have a high-level understanding of the available resources and how to use them to get the info that [they] need for OSPR management. But [we] don’t have a detailed list of things that are currently missing, redundant, or maybe even inconsistent. So as a next step it would make sense to have a closer look and document that.

Review of existing resources

Open edX wiki

CC Program

Main resource: Core Contributors to the Open edX Project

  • Comprehensive list of current (and past) Core Contributors.
  • Documents write access to repos for CCs that are Code Contributors, but format isn't standardized.
  • ...
Maintainership Pilot

Main resource: Cumulative Maintainer Responsibilities

...

Open edX Backstage

https://backstage.openedx.org/

...

Other resources

  • 2U spreadsheet with code ownership mapping (private, shared with OSPR managers)

...

Potential solutions

TBD

Changes applied

TBD

antoviaque commented

@itsjeyd How are things going with this issue? Anything blocking I can help with?

Note that I don't know if we currently track product review delays as part of the current ticket, but delays there too have been mentioned by Nacho in the last Campus meeting: https://otter.ai/u/IkmOT8IcZ7XWmO3x_DZBVeg1HyU?tab=comments&view=transcript&t=1487s -- if it's not in the scope of the current ticket, it would be worth creating a ticket to track it.

itsjeyd commented

@antoviaque Thanks for checking in! There are no blockers per se, I simply haven't found time to continue working on this issue yet.

And you're right, product review delays aren't in scope here, and creating a separate ticket for addressing them would make sense. It sounds like there is consensus that the process for product reviews is due for an update. So if we could find someone with the necessary bandwidth to drive that conversation forward in the coming weeks/months, it should be possible to make some improvements.

CC @mphilbrick211

mphilbrick211 commented

Hi @antoviaque and @itsjeyd - Product has started a new process (very recently) where they are trying to catch and review product-related contributions before they go onto the Contributions board at all. At the moment, here are the current items they're reviewing.

Once product is done reviewing an item, they will move it to the Contributions board with a label saying "Product Review Complete" (we haven't created that label yet as this is still brand new). This way, once it's on the board, we know it's waiting for Engineering only. Given that this is something we're trying out, it might be a bit rocky for the first few months until we get a good rhythm going.

There are still a few stalled product items from over the past several months, but some of them require lots of extra discovery work, so those PRs have been moved to drafts until it's more appropriate to move forward with them (so, intentionally paused, not stalled).

@itsjeyd - can you let me know which Product items are stalled on your side?
@antoviaque - are there any you're worried about that are stalled that maybe I can help get you an update on? I'm with Jenna in the office this week, so I can ask.

itsjeyd commented

@mphilbrick211

can you let me know which Product items are stalled on your side?

Will do. I have my next round of triage coming up on Thursday.

  • 👍1
antoviaque commented

@mphilbrick211 Thanks for the update on this! And for the product reviews that are stalled, Jenna would already be aware since Nacho brought that to her in the Campus meeting, so I think that's good, but I'll let you know if I hear about another, besides what @itsjeyd would already know about.

  • 👍1
jalondonot commented

20/02 Contributor's meetup update:

  • The survey results will be reviewed this week
antoviaque commented

Referencing the discussion from https://openedx.atlassian.net/wiki/spaces/COMM/pages/4128014347/2024-03-21+Meeting+notes?focusedCommentId=4130308114 which is related to this ticket : "Should 2U PRs go to the Contributions Board?"

  • 👍1
antoviaque commented

@itsjeyd To follow-up on the discussion from #104 (comment) on the ticket that currently tracks this type of improvements:

The issue that I mentioned above (= no mapping between repos and CCs) is still open, though. I had posted about it internally before the conference but there were no takers so far.

Did you end up getting the help you need?

itsjeyd commented

@antoviaque Yep, we have a discovery ticket that @xitij2000 is working on. He has already come up with a potential solution and posted about it in the #wg-maintenance channel on Slack to get feedback on it. CC @mphilbrick211

Thanks for checking in! :)

  • 👍1
antoviaque commented

@itsjeyd Great, thanks! 👍

@xitij2000 I see that you didn't get many answers on Slack - it could be worth posting it in the forum too - I don't think everyone sees it in Slack, which is more synchronous. You should also ping specific people that you want feedback from on this - and maybe attend a maintainers working group meeting to ask for reviewers. Shout if you need help.

xitij2000 commented

@itsjeyd Great, thanks! 👍

@xitij2000 I see that you didn't get many answers on Slack - it could be worth posting it in the forum too - I don't think everyone sees it in Slack, which is more synchronous. You should also ping specific people that you want feedback from on this - and maybe attend a maintainers working group meeting to ask for reviewers. Shout if you need help.

The meetings are at a difficult time for me, but I was planning on attending one since I didn't get any responses.

I'll post on the forum first and see if that fares better.

  • 👍1
itsjeyd commented

@antoviaque @xitij2000 I'll be attending maintenance working group meetings again from next week on, so should be getting an opportunity soon to raise the topic there.

The more places we bring this up, the better.

  • 👍1
xitij2000 commented

@itsjeyd Thanks! I think the general impression I get from the forum discussions is that we should automate adding this to catalog-info.yaml and then from there it can be pulled into Backstage. This will allow anyone to open this file from the repo and see the list of contributors.

What I liked about the project board is that it allows one to quickly filter based on either the repo or the contributor, however, I can understand how it might not be ideal either since project boards aren't really designed for this purpose.

itsjeyd commented

@xitij2000 I just replied to the conversation over on the forum thread -- maybe there's a way to integrate CC info into the Contributions board? That would be a bit of a game changer for OSPR management.