Add Core Committer Reviewer Field to Jira?

@antoviaque @jill @usman What do you think about adding a separate field in JIRA for “Core Committer Reviewer”?

I ask because:

  • Lately there have been quite a few tickets that have asked for me to do the upstream review as core committer, but I can’t actually assign myself because they already have two reviewers. e.g. https://tasks.opencraft.com/browse/SE-3405
  • This would make it simpler to track core committer work; right now I have to search for tickets where I’m a regular reviewer and it has the “CoreCommitter” label.

I’d also be curious to hear your thoughts about adding “CC Review” as a separate status for tickets, since it’s sometimes unclear when tickets are undergoing CC review or not, although I’m reluctant to add any new status columns to our Jira boards.

6 Likes

A “CC Reviewer” field is a good idea @braden.

To avoid cluttering up the Jira Sprints board, could we create a sub-column under Internal Review for CC Review? IIRR @antoviaque did this for the Refined status at one point while we were sorting out the async sprint planning.

Then there’s the question of spillover – are we expecting that CC reviews will always happen within the current sprint? I know that’s the goal, but it does constrict the ticket timelines, and if there are several review cycles, it can be hard to plan for. If a ticket is in CC Review at the end of the sprint, can it be like External Review, and not be spillover?

2 Likes

Agreed, currently there’s no defined process for ticket workflow for tickets with core committers assigned, so it will be good to define and document it. :+1: I’m not too fussed with exactly how, but I echo @jill’s comments above.

1 Like

+1 to having a dedicated field for the core committer reviewer.

I would be fine with an extra status column. Imho the core committer review replaces the upstream review, and one of the main things I’m eager to get from it is to eventually stop having 5000 tickets that span multiple sprints and lay around in the “external review” column. In Scrum tickets are meant to be completed during a single sprint, and it would allow us to go back to that. So I would want to see that status be used similarly to the “review” status, including for spillovers. Actually, once we’ll all be core committers, “review” and “core committer review” might actually be a single step – which would help streamlining the timeline.

@braden It could be worth planning a task to make these changes to Jira & update the process in the handbook? Do you need any help to make the changes in Jira, or do you already know how to do it?

I am perfectly fine with any of the above recommendations. :slight_smile:

Thanks for the positive response :slight_smile:

I have gone ahead and created a “Core Committer” field. There seems to be a new option in Jira to create a field that allows selecting multiple users, so I’ve gone with that for now. (There have been occasional tickets where @jill and I are both core committers on the same ticket, reviewing PRs in different repos.) Let me know if you think that’s good, neutral, or bad compared to a single core committer assignee. The only disadvantage I can see is that the form doesn’t look as nice.

Here is an example with one CC: https://tasks.opencraft.com/browse/SE-3405
Here is an example with two CCs: https://tasks.opencraft.com/browse/SE-3381

Here is an example search for assigned CC reviews: https://tasks.opencraft.com/issues/?jql="Core%20Committer(s)"%20%3D%20bradenm

Ticket for adding CC field (in progress already): https://tasks.opencraft.com/browse/MNG-1892
Ticket for future “CC Review” status in Jira (I’ll do this later), and updating the handbook to clarify the process: https://tasks.opencraft.com/browse/MNG-1893

4 Likes

@braden Thanks for doing this!

For the type of field, sorry to be the naysayer here, but I actually feel pretty strongly about assigning things to multiple people at once - when more than one person is responsible for something, it dilutes responsibility as we naturally tend to assume the other people will be able to pick up the work. That’s why even in cases where we can’t avoid having multiple people, like reviewers, we have “Reviewer 1” and “Reviewer 2”, each with a specific type of responsibility (ie, the reviewer 1 is responsible for doing the review - reviewer 2 is only an extra bonus on which the reviewer 1 can’t count on to do his part or delegate his work to).

If you really need to be able to assign multiple core committer reviewers, we could use the same principle though.

@antoviaque We would only use that in cases like the example I gave, where Jill can review the configuration PR (which I’m not supposed to review), and I can review the edx-platform PR (which Jill isn’t supposed to review). So there’s still a clear responsibility for each person.

But those situations are pretty rare so it may be fine to just have a single person field to cover 95% of cases?

It’s not that rare… e.g. all our ORA2 PRs have had associated edx-platform PRs, and the more MFEs we build, the more repos will be involved there.

Can we keep the multiple-user field type, and specify in the task description who is associated with which PR?

@braden @jill As mentioned above, I really want to avoid the dilution of responsibility that goes with multiple assignees fields. We can do two different fields like for reviewer 1 & 2, and in cases like this keep the “core committer reviewer 1” as the main core committer responsible for the whole review, including pinging the second reviewer if there is something they can’t do themselves?

1 Like

OK, no problem. I have changed it to “Core Committer” and an optional “Core Committer 2” when necessary.

2 Likes

@braden Perfect, thank you!

I have now added a Core Committer Review status column. It will show up in on boards in the same column as “Internal Review”, since it’s a type of internal review. I will be updating the handbook to document this clearly and then do an announcement; ticket is MNG-1893. Until then, please help me “beta test” this :slight_smile:

3 Likes

@braden I think we’ve missed factoring the edX product review aspect into the “no-blockers” workflow for upstreaming tickets.

For feature tasks (such as the ones being done on ORA and LTI epics), we need to get a product review from edX before going for the engineering one, and that’s usually what takes more time.

We should still move those tickets to External review/blocker, right?

If not, should we look at getting product reviews first (before upstreaming/implementing the feature)? That approach didn’t really worked out with edX when we tried to use PDPs (development proposals) and we got little feedback on the submitted proposals.

While the edX projects might not be impacted by this (they want to review and deliver faster), our contributions might take longer to get an approving review.
Any ideas on how to speed up that process?

I haven’t forgotten about it. If you check out the handbook PR, you’ll see it’s a known part of the flow. I’m not calling this a “no-blockers” workflow for that reason - there’s still a big blocker in the middle, if it introduces new features or changes the UI/UX.

Yes, still move it to External Review/Blocker during the product review phase.

Yes: Nimisha has expressed a desire to grow the core committers program beyond just technical reviews, to include things like documentation reviewers, UI/UX reviewers, and hopefully also product reviewers. So that’s one possible way.

Another is: try to get the edX product review done early; it can happen even before any coding starts, if we open a proposal. Though I know we’ve tried this before with limited success. It also cannot be too much before or they may “retract” their approval later, if things change :/

We’re also discussing streamlining the overall process a bit, removing the internal review if there’s a core committer review anyway.

1 Like

Thanks for clarifying that!

And it’s good to see that the CC role is expanding to address those points and “unblock” more community contributions. :slight_smile: