Advancing with the Triage & Review Team idea.

161 views
Skip to first unread message

Carlton Gibson

unread,
Oct 3, 2020, 3:53:46 AM10/3/20
to Django developers (Contributions to Django itself)

Hi all.

With the new Technical Board elected (Well done to everyone there!) we have an important workflow that’s not covered by DEP 10. 

Specifically, this scenario: 

- An issue comes in covering some esoteric corner of the ORM — generation of column aliases for group by clauses say. 

- Mariusz picks this up and opens a PR fixing said issue. 

- Simon reviews PR and approves it. 

- Mariusz merges. 

Up until now this has been covered by Simon’s role on the TB as, under DEP 10, TB members are allowed to approve a Merger’s PR. Officially the PR here would now require further approval from Claude or myself before Mariusz could proceed. 

This is a symptom of a wider issue. 

Frequently, for example, Marisuz makes a small PR on the weekend. I'm not working Monday, so it has to wait until Tuesday morning for me to see it and Approve it.

Meanwhile one or more regular contributors have taken the time to look at the PR and approve it.

A couple of actual examples:

1. Made indexes tests use required_db_features.
https://github.com/django/django/pull/13432
Approved by Hasan and Tim.

2. Corrected docstring quotes in various code.
https://github.com/django/django/pull/13445
Approved by David and Hasan .

David, Hasan and Tim are all regular features on PRs, Trac, and here, making PRs, reviewing, commenting, helping to triage and so on. Tim is Django's #1 all-time contributor. Hasan has made nearly 200 commits and it there every week taking on issues from the backlog. David is a newer arrival, has been super active and offered good input across the board, both here and on third-party packages. 

It seems clear that their approval should be enough for a Merger PR to progress. 


Proposal:

Some time ago I proposed a Triage and Review Team.

https://groups.google.com/g/django-developers/c/mUBWlG0-Jbw/m/0dtgwMwPAAAJ

I began this but it ran into the DEP 10 changes, and I stopped working on it to let that go through. With the new TB in place, I'd like to bring that back now, granting PR Approval power, at least for Merge PRs.

The idea was to have a team for folks that are active on repo. This would provide some recognition for efforts, allow extra hands to help manage tickets — closing spam ones seems relevant this week… 😃 — and so on. 

The GitHub Team I created has GitHub’s Triage role. This allows for one thing to request a review from a Merger. Nick is the only member of that team currently (see above about pausing) but it’s worked well when he's reviewed PRs from the backlog, seen that they're ≈ready and asked for a review from me. This has been very helpful, and quite smooth. (It’s in a similar vein to marking tickets on Trac Ready for Commit, which also helps them to show about the mass.) 

Ultimately I think a Triage & Review Team is an opportunity to widen the contributor pool and spread the work.

I would suggest the following list for members to flesh out the team initially. Happy to add more, but the idea was currently active (if you're offended I missed you, sorry, do shout! — not intentional. ) 

- David Smith

- Hasan Ramezani

- Jon Dufresne

- Nick Pope

- Simon Charette 

- Tim Graham

(I didn’t include Mergers or TB members.)

Some other points: 

- I’d like to start measuring non-commit contributions somehow. This is something I’m thinking about but for now I think by-eye is sufficient. 

- I’d be happy to make the list twice as long. 

- As I say, currently active — we should review each cycle. 

- It’s still all men — but the team would provide a way of recognising anyone finding Triage as a Hobby appealing, so I’d hope it would act as an incentive to participate, with a clear path to recognition (and from there candidacy in a TB election say…)

Hopefully that’s clear enough. Can I ask for thoughts, and if we’re keen, what would we need to do to make it formal?

Thanks all. 

Kind regards, Carlton



charettes

unread,
Oct 5, 2020, 9:46:53 AM10/5/20
to Django developers (Contributions to Django itself)
I like the idea but I don't feel comfortable vouching for it given my conflict of interest.

I'll just say that It does feel weird that folks on this list which are the main active contributors outside of a few members of the currently elected TB don't have access to basic triaging and reviewing features on Github. It feels like these permissions would be way more useful to them, and the active contributors members of the TB that are not mentioned here, than to members of the TB themselves given the rare times they have to step in on a Github issue (mainly DEPs? so far).

Cheers,
Simon

Carlton Gibson

unread,
Oct 5, 2020, 10:33:15 AM10/5/20
to Django developers (Contributions to Django itself)
Thanks Simon. 

There are two parts to this: 

* the team, with the Triage and Review permission. 
* Whether they can approve Mariusz’ PRs over the weekend. (And other times…)

I could take the previous discussion for a Yes on the first part, and just invite more people. (I should probably just do that.) 

BUT I’d like to resolve the second point here (it’s needed for the examples given).
 
I think it might need an edit to DEP 10… as per here:

If so, I’m happy to type that up but I guess some pointer from the new TB that it would be acceptable would be good. 

Kind Regards,

Carlton




-- 
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/c34045c8-55fa-4073-b64e-20f2d719d12an%40googlegroups.com.

Tim Graham

unread,
Oct 5, 2020, 8:49:35 PM10/5/20
to Django developers (Contributions to Django itself)
I'm in favor of the proposal. It seems good to empower the people doing the work and remove some red tape regarding such a limited number of people who can approve a PR. I don't think Simon's approvals should carry less weight merely because he wasn't re-elected to the technical board. Now that the technical board election is open to the entire DSF, most people voting probably don't follow day-to-day Django development anyway -- I can't say that I do. Actually, I haven't even followed DEP 10 close enough to know that my approval is no longer sufficient for Mariusz to merge his own rather trivial PR, or that in voting for the technical board, I was voting for who could approve PRs. It seems needlessly restrictive.

Carlton Gibson

unread,
Oct 6, 2020, 4:43:53 AM10/6/20
to Django developers (Contributions to Django itself)
OK, I've added Jon, Simon and Tim to Nick on the T&R team.

That has GitHub's Triage permission, so you can do various Triage-y like things to PRs.
(Possibly you already could given membership of other teams but...)

I've also invited Hasan and David. They'll need to accept, as not already Django org members.

This on the basis of the previous discussion, and no objections here.

Thank you all for your contributions and welcome aboard Hasan and David, in particular.

I'd like to extend this further, but that'll do for today.

I'll have a look at an adjustment to DEP 10 later on. I'd like to push that change forward.
Thanks all.
C.

Carlton Gibson

unread,
Oct 6, 2020, 6:31:33 AM10/6/20
to Django developers (Contributions to Django itself)
First pass at a PR to DEP 10 https://github.com/django/deps/pull/71
Reply all
Reply to author
Forward
0 new messages