PR Workflow Evaluation and Interventions [PR]

[jmakowski1123]

Acceptance Criteria

The goal is to arrive at a list of recommendations for where and how the Product WG can intervene to remove bottlenecks in the PR review process and to ensure timely reviews.

Product Brief

https://openedx.atlassian.net/wiki/spaces/OEPM/pages/3476357132/Idea+Board+Project+Briefs#8.-PR-Workflow-Evaluation-and-Interventions

Child Tasks

• Review the steps where product reviews are expected in the OSPR process
• Evaluate the current practice by reviewing the tickets currently needing a product review, and by asking the community for feedback about them
• Trial run
• Improve the product review practice & steps; in particular, figure out a way to ensure contributions don’t end up blocked, along with detection & escalation of any missing product review

[github-actions]

Thanks for your submission, @openedx/open-edx-project-managers will review shortly.

[antoviaque]

@jmakowski1123 @sarina I have started looking into this:

1. Reviewing the OSPR product steps

Review the steps where product reviews are expected in the OSPR process.

As far as I understand, the main page describing the OSPR process and its integration with product review is on the Open edX Developer's Guide - correct @sarina? Are there other places where this is described?

Here are the relevant sections I've spotted in there:

https://edx.readthedocs.io/projects/edx-developer-guide/en/latest/process/community-manager.html

"Help the product team evaluate the idea behind the pull request. Is this something that Open edX wants? If you and the product owner(s) all believe that Open edX does not want this pull request, add a comment to the pull request explaining the reasoning behind that decision. Be polite, and remind them that they are welcome to fork the code and run their own fork on their own servers, without needing permission from edX. Try to suggest ways that they can build something that Open edX does want: for example, perhaps an API that would allow the contributor to build their own component separately. Then close the pull request."

"The community manager should make sure the pull request is ready for Product Review, if that has not yet happened. That means getting enough detail out of the contributor for the product owner to properly do a product review. Once this is done, move the JIRA ticket to the “Product Review” state. If questions arise from product owners during review, work with the contributor to get those questions answered before the next round of review."

_"The product team will meet biweekly to review new proposals and prioritize PRs for team review."

"Once a PR has been prioritized for team review, ask the product owner for an estimate of how many sprints it will take for the pull request to be reviewed: if its more than one, try to push back and advocate for the contributor. However, the estimate is ultimately up to the product owner, and if he/she says it will really be more than one sprint, respect that."

https://edx.readthedocs.io/projects/edx-developer-guide/en/latest/process/product-owner.html

"The product owner has two main responsibilities: approving user-facing features and improvements from a product point of view, and prioritizing pull request reviews.

When a contributor is interested in developing a new feature, or enhancing an existing one, they can engage in a dialogue with the product team about the feature: why it is needed, what does it do, etc. Product owners are expected to fully engage in this process and treat contributors like customers. If the idea is good but the implementation idea is poor, direct them to a better solution. If the feature is not something we can support at this time, provide a detailed explanation of why that is.

Approving work involves more than just giving the idea a go-ahead. There are a number of factors to consider.

• What level of support will edX provide? Unsupported, provisional, or supported?
• Will edx.org use the feature? Does it need configuration support?
• How much documentation is needed for the feature? Will edX write the documentation, or should the contributor provide it?
• Does the work require other review, such as user experience, design, accessibility, internationalization, training, or customer support?

The earlier in the process these other roles are involved, the better the process will work, and the better the final product will be.

A product owner is responsible for prioritizing pull requests from contributors, and keeping them informed when prioritization slips. Pull requests that are ready to be prioritized in the next sprint will have a “Awaiting Prioritization” label on their JIRA review tickets. At every product review meeting (which should happen each sprint), pull requests awaiting prioritization should either be included in the sprint for the appropriate team as a commitment to get the pull request reviewed, or the product owner must inform the author of the pull request that the pull request is still queued and is not being ignored. Contributors should be treated as customers, and if their pull requests are delayed then they should be informed of that, just as a product owner would inform any customer when that customer’s requests are delayed."

2. Review current tickets needing a product review & ask for community feedback

Evaluate the current practice by reviewing the tickets currently needing a product review, and by asking the community for feedback about them.

@sarina Is there a way to list the tickets which are waiting for a product review in https://github.com/orgs/openedx/projects/19/views/1 ? It would be useful to be able to keep an eye on that set of tickets.

I know of the following tickets, but there are likely more in the queue:

• Increase grading rounding precision
• Change schedule datetime inputs in studio to user timezone 2

Also, this isn't a PR, but there is the product review of Allow setting due dates for an entire course section which is still pending (there was also PDP-8 for it, but this URL seem to have vanished/moved?). The initial product review request dates back to September 2018. :) Then the PDP was created in 2019 at edX product's request I believe? Not sure if there is a current replacement for this type of product review request.

If we ask for the community's feedback, we might also uncover more cases like this. I could create the forum thread for it.

[sarina]

@antoviaque - let me know if I can answer anything further (we spoke briefly about this at PWG today)

[sarina]

@antoviaque -- Jenna suggests that I spend some time helping you get this ticket to completion. How could I best help out?

[antoviaque]

@sarina @jmakowski1123 Thank you for the help on this! I think one of the important steps currently is to develop a good understanding of how and when the product reviews are conducted currently on OSPRs - that will help to guide figuring out fixes.

@sarina What are in your experience the main blockers, in terms of getting the product reviews assigned, and completed quickly? What are the frequent pain points?

That's actually also related to my next steps/tasks:

• We need to ask for broader community feedback on this, to check whether there is any additional pain points that we haven't identified yet. I have created a feedback thread for this in the official forum, linking back to the current issue for context: https://discuss.openedx.org/t/product-reviews-on-osprs-looking-for-feedback-about-the-process/8152

• During the last product working group meeting, we agreed that it would be useful to get your review on this @natabene - you manage the OSPR board, so you will have unique insights on this, and might already know the answer to points discussed above (like how to display the list of tasks waiting for product review, for example) - would you be able to have a look?

[natabene]

@antoviaque I am happy to help, but I need clarification. Can we chat?

[antoviaque]

@natabene Thank you! And happy to chat yes -- do you want to pick a time in my calendly?

[natabene]

@antoviaque Done

[natabene]

@antoviaque Cannot find the meeting link, do you mind sharing it via Slack?

[natabene]

Here are my thoughts:

• I am not sure who currently owns this document. It has not been updated in years and is doesn't reflect what is happening in reality when I triage.
• After edX became part of 2U, the role of community manager has changed. Previously, edX owned all repos in the platform -> all contributions coming from outside of edX were called OSPRs -> edX needed to review them all. However, now that I am part of 2U, my triage management is 2U-centric. All PRs that produced by authors not associated with 2U are referred to in my board as External Contributions, as in "external to 2U".
• This means that as a 2U triage manager I triage and focus on getting reviews only for contributions that are made to repos maintained by 2U.
• Currently the board still catches PRs to repos, maintained by other companies/organizations, too, but this is a bug, not a feature. Once there is reliable way for the OSPR bot to automatically determine via a readme file or some other way maintainer of each repo, I will tweak my board to only display external PRs opened against repos maintained by 2U.
• In a nutshell, I am a 2U triage manager, that happens to triage also all other PRs from community for now, but in the future ideal state I would only be triaging open source PRs agains only repos maintained by 2U.
• Even within 2U and using our internal processes we struggle to get product review in a timely manner. There is not set process, each team within 2U has their preferences that triage manager tries to accommodate. For tracking purposes, I used to have an additional state "Product Review", and kept ticket/issue in that state until product has signed off. Then it would go to engineering review. However, in my current Github board I don't have this state for simplicity's sake.
  CC @nedbat

[sarina]

@sarina What are in your experience the main blockers, in terms of getting the product reviews assigned, and completed quickly? What are the frequent pain points?

My knowledge is out of date (from 2013-2016). At that point in time, it was always difficult to get anyone from edX product team to sign off on any community change, resulting in OSPRs that would sit for weeks, months, or years. @natabene - is anyone from 2U product required to sign off on changes, and if so, how long does it take them? One point of this ticket is to figure out how we could shorten this process by allowing community product team members decide within the community which changes the community wants in the product.

[natabene]

@sarina I am not aware of a requirement for product managers at 2U to sign off on community changes, especially within specific time frame. It happens ad hoc.
On the other hand, engineers from the same 2U team will not merge something that requires their product manager's approval, but has not received it yet. Such PR will wait until and if product manager gets to it.

[sarina]

@natabene so you answered my question - things don't get merged until a Product Manager signs off on it. And that may or may not happen.

That is unacceptable to our community, and we need a better way forward.

[antoviaque]

@natabene @sarina Thanks for the details! It's really useful to be able to know of where things stand today. And that blocker is precisely something that the transition from the split between Open edX and edX/2U gives us an opportunity to fix - I think it will be better for everyone involved to have a better solution to handle those cases.

Since the rules here would ideally apply the same way for all community members, it's probably useful to base some things off who is the maintainer of a repository. Here we are discussing about repos/features where edX/2U is the maintainer, and handles product management on it. As more organizations and individuals take on maintainership on official parts of the Open edX project, similar considerations will apply, but instead of edX it might be contributions & product reviews to features maintained by eduNext, OpenCraft, etc.

And it's probably useful to separate 2 different scenarios for the maintainer and/or the product manager assigned to the project receiving a contribution, depending on whether the maintainer/product manager of the repo is:

• A) interested in the change, and is able to provide a product review quickly (2 weeks?)
• B) not interested in a change, or not able to provide a product review quickly

In case A) things can probably continue as they are now, maybe with the an initial routing of the OSPRs on the Open edX github org, rather than edX', to triage issues within the project before it gets to the edX/2U ticket system & processes?

For B), it's still good community practice to still review and merge useful changes even when they are not a priority -- contributions are rare and precious, and volunteer work need to be cultivated. But the main requirement maintainers currently have for work that they aren't interested in is "the ability to promptly triage incoming requests that propose changes to or extensions of the component, assessing their appropriateness and/or routing them to proper reviewers". (cf OEP-55) But not to accept them, or even to review them fully.

So we mainly need to figure out what to do in such a case. Imho:

• The maintainer / product manager assigned to the project should decide whether to say "no" to the feature -- if not things stop there, and the contribution is refused
• If not, then another product manager core contributor from the community can volunteer to do the product review, and their 👍 or 👎 will be binding like if it was the main product manager / maintainer.

Since now ultimately the product decisions' final say lie with the Open edX project, decisions by the product manager/maintainer of the repo (whether from edX or anyone else in the community) would also be subject to the escalation path from the product management workgroup charter imho -- see the discussions about this at https://openedx.atlassian.net/wiki/spaces/COMM/pages/3487301637/Product+working+group+Charter?focusedCommentId=3497820190 . So in all cases, if anyone is unhappy with the product decisions, as for the rest of the product working group decisions, after discussions they could be escalated to tCRIL product management, and in last resort to the TOC?

CC @jmakowski1123 @e0d

[sarina]

I think we need buy-in from the 2U product team on anything that imposes requirements & deadlines on them. I think @jmakowski1123 is likely in a good place to begin these conversations.

I do like the idea of an escalation path if a review is not happening promptly. I know in the past, the 2U team has had a difficult time even saying "no" to a contribution.

[antoviaque]

@jmakowski1123 @sarina Ahead of the upcoming product meeting, do you know if there have been new developments on the topic of product reviews since we last discussed here? If so, it would be useful to review & comment async, that would make the next meeting discussion more efficient, and allow others to follow it here.

FYI there is another example brought up by @mgmdi in the last core contributor sprint retrospective and discussed with @jmakowski1123 & @sarina on Slack.

CC @mphilbrick211 @itsjeyd as this is related to the management of incoming contributions that you're currently reorganizing.

[jmakowski1123]

@jmakowski1123 @sarina Ahead of the upcoming product meeting, do you know if there have been new developments on the topic of product reviews since we last discussed here? If so, it would be useful to review & comment async, that would make the next meeting discussion more efficient, and allow others to follow it here.

FYI there is another example brought up by @mgmdi in the last core contributor sprint retrospective and discussed with @jmakowski1123 & @sarina on Slack.

CC @mphilbrick211 @itsjeyd as this is related to the management of incoming contributions that you're currently reorganizing.

@antoviaque I just touched base with Ryan about an agenda this morning. Were thinking to nail down:

• Clarify where in the current OSPR process the specific product review step falls
• Clarify how it is currently tracked
• Decide who should be involved in product triage and when. Let's define the criteria for when a PR needs product review, who it should go to, and where Prod WG can step into that process in a meaningful way.

Does this sound right to you? What would you add or change?

[mphilbrick211]

@jmakowski1123 @antoviaque while the product issue is being decided, in the meantime, would it be helpful for @itsjeyd and I to know about any high-priority items we should look out for that you know will require a product review? That way we can at least get some idea in case there's things in flux needing a product review.

[jmakowski1123]

@jmakowski1123 @antoviaque while the product issue is being decided, in the meantime, would it be helpful for @itsjeyd and I to know about any high-priority items we should look out for that you know will require a product review? That way we can at least get some idea in case there's things in flux needing a product review.

@mphilbrick211 I think that's part of the challenge, that there is no definitive list, at least that I know of. In terms of criteria, my initial thought is a PR would need product review if the changes affect the end-user experience in a noticeable way....things like changes to authoring flows (I would think anything in Studio, really?), learner experiences, admin experiences.... And ideally we can bake a requirement for this information into the PR when it's submitted, so it can be easily identified and triaged...

[itsjeyd]

@jmakowski1123

In terms of criteria, my initial thought is a PR would need product review if the changes affect the end-user experience in a noticeable way....things like changes to authoring flows (I would think anything in Studio, really?), learner experiences, admin experiences...

Thanks @jmakowski1123, I think this provides a good starting point for identifying PRs that should get a product review when checking their status in the context of CC project management.

[antoviaque]

@jmakowski1123 Thank you! That sounds like great points to sort out 👍

For the last point, it could be useful to break it down further, and make some of the goals embedded in it explicit:

Decide who should be involved in product triage and when. Let's define the criteria for when a PR needs product review, who it should go to, and where Prod WG can step into that process in a meaningful way.

Based on the previous discussions, I see those ones:

• Define specific expectations of timeline for the different steps of the process, so that both the contributors and the reviewers can plan & schedule work reliably; ie how much time triage/assignation would take, maximum time for the reviewer to complete the review, maximum time for the contributor to implement the requested changes, etc.
• Decide how to handle cases where reviewers or contributors are not able to allocate sufficient time for the reviews (or the requested changes) to happen -- it could be the escalation process we discussed above, closing the PR outright, etc. In any case, I think the main goal here would be to eliminate the cases where a contribution remains hanging for weeks, months or even years for a product review -- which is one of the main issues currently.
• Figure out how to handle cases where there are disagreements between the contributors and the reviewers -- that could simply be to apply what we described in the product working group charter (ie aim to work mainly by consensus, but with possible escalation to product core contributors & tCRIL product management/you, and the TOC as a last resort) -- but we need to make sure we all agree on those principles now, rather than coming as a surprise for anyone if/when it happens. Sometimes emotions can ramp up in those cases, so it can be good to set the expectations.

Btw, do you know Ryan's github handle? It might be useful to loop him in on this type of thread.

[sarina]

@antoviaque I think it could be helpful to make a proposal doc with your personal recommendations about how to answer these questions - we can get review and input from others. I find that's often a faster way to bootstrap discussions and solutions.

[antoviaque]

@sarina Good idea, this sounds like a good next step to formalize it. Today we've had a good conversation with Ryan and @jmakowski1123 during the product working group, and Ryan took on an action item to discuss the proposal of passing on product reviews to product core contributors when there is no capacity on the 2U side -- I'll wait to hear back from him on how that went, and if it looks like a good approach I'll flesh it out on a document. 👍

[jmakowski1123]

Notes and next steps are recorded here: https://openedx.atlassian.net/wiki/spaces/COMM/pages/3561324672/2022-10-25+-+Product+Working+Group+Meeting+Notes+and+Agenda cc @sarina @antoviaque @mphilbrick211 @itsjeyd

Corresponding tickets for next steps are:

Formalize step to create an issue (in relevant repo) with each PR

• Identify 5-10 beta repos to trial, preferably ones that have open PRs that require product input
• Pair with Feanil to add how-to documentation that outlines the new step
• Create communication channels to test drive and iterate
• Pair with Michelle to build workflow to identify which issues warrant roadmap on regular cadence

Write formal guideline for when EMs should get product review on PRs

• -Communicate these guidelines to project managers and repo owners

[antoviaque]

FYI, there is a new document from Santiago (not sure what his github handle is?) laying out a proposed approach for conducting product reviews: https://docs.google.com/document/d/1rHLCLxMXzOQ0Iwn-75FRJk4tC-5JTiE2G-bmA0sSISA/edit#

@jmakowski1123 Are the list of steps you outlined out above still the next items, or has the list evolved?

[jmakowski1123]

@antoviaque I think the steps outlined still apply, and Santiago's doc will help to flesh out/inform what actually goes in the repo documentation. The way I'm interpreting it is: The steps above focus more on the "how" do we get more transparent (and earlier) product information about new features in the repos and on the roadmap, while Santiago's doc and the comments you've made above in this ticket focus more on "who" has what role/responsibilities and "what" info needs to be included for a good PR. Does that align with how you interpret it?

I think it makes sense to get consensus on Santiago's doc first, but I've also added the above steps to the docs.openedx.org sprint calendar so we can action it in a timely way: openedx/docs.openedx.org#223

[antoviaque]

@jmakowski1123 Sounds good to me, yes! 👍 I'll do a pass of review on Santiago's document.