Thanks @ewa.jodlowska for the help in getting this set up.
Python org is now a non-profit org, and we’re in the “Team” plan, and we can start creating the Triagers team.
Can anyone here come up with a list of people I should add as triagers on GitHub? I think we should try this with a small group of active and experienced triagers, and work out any issues that may come up.
While that is happening, I will work on adjusting miss-islington and maybe bedevere to take the triage role into account.
Update devguide re: the new Triage team, their role and expectations
Of 49 members of that list, 28 have set their GitHub username. If that’s more than we want to start with, there are several names in that list that have been quite active lately that could make a good starting set.
I think that’s a reasonable starting set, but that anyone on the list should feel free to request being added to the triage team even before the full group is added; all should be added by the time PEP 588 is implemented.
No problem, thanks for the clarification. Based on the name and my impression so far of the current triagers, I had thought they were more focused on prioritizing individuals issues and on the submission of the patches themselves rather than reviewing PRs. I have not yet seen too many triagers in the PRs of others on github assigning labels, but that might just be because the role is quite new and there’s not too many of them yet.
As someone who has recently taken a particular interest in the code reviews specifically, what would it take to become a triager? Also what is the expected experience level of the role? I’ve come to understand that the core developers should be reserved to the veterans of the Python community, who have had significant involvement across hundreds of topics in their respective areas.
Edit: I’m not at all suggesting to be on the immediate list of candidates since I’m still new to the Python dev community (had my first commit in June). I just wanted to get a rough idea of the expected experience for a triager so that I had a goal to work towards.
On the issue tracker it’s someone who has consistently shown they know how to respond to issues. I suspect the same will be done for GitHub and PRs: basically someone who has built up trust with a core dev to warrant giving them partial control of the project.
Before we start giving triage permissions, shouldn’t we first define what triagers are allowed to do? For example, I suppose they should not apply the automerge label (even if they technically have permission from GitHub to do that). Should they be allowed to set needs backport labels? Should they be allowed to set awaiting merge if they agree with a PR or awaiting changes if they don’t agree? Should there be some kind of mentoring for triagers?
If self-nominations are accepted, I would like to apply for triager permission. I mostly work on the core interpreter and C API, I’m co-author of PEP 590. See my bugs, my PRs and my reviews.
Not that my vote is of particular importance here, but I would recommend jdemeyer. He’s been highly responsive to feedback/questions (even to those with minimal C experience, such as myself) and I constantly see him submitting impactful performance improvement PRs. Performance optimizations are far from the only way to meaningfully contribute, but this seems to be his specialty and as far as I can tell, he does it well.
Submitted a PR as an inital draft for the Bug Triagers section (added to the existing “triaging” page on the devguide): https://github.com/python/devguide/pull/506. Feel free to submit feedback or suggestions, this is still a WIP.
In addition to the proposed expectations for the triager role, I would also recommend for a slight rework of the awaiting review and awaiting core review labels. At the moment, if anyone submits a review on a PR with either an approval or change suggestion, the awaiting review label is automatically removed and the awaiting core review label is added.
In order for these labels to be more helpful for improving the workflow of the core devs, I would recommend restricting the changing of these labels to only apply when a triager does a review of the PR (once the triager role is fully implemented on GitHub). This would make the label more useful to the core devs, as they would know that someone who was proven they are capable of providing constructive feedback has approved of the changes.
I would also suggest for the label to only be changed when the PR is explicitly approved, or if someone manually adds the label (when the input of a core developer is required). In many situations, I’ve suggested that rather straightforward changes should be made (which wouldn’t require the attention of a core dev), and then the label is inappropriately changed to awaiting core review automatically.
That’s partly why I was making the other recommendation for the ability to change the awaiting review to awaiting core review to be restricted to the new triager role or higher. They wouldn’t be able to block the PR (even if they requested changes), the label would just stay on awaiting review instead of changing. Currently, the same thing occurs if a reviewer chooses the middle option, which leaves feedback without explicit approval or change request. The suggestions the triager made may not align with the core devs, but it is likely to be more aligned in comparison to someone who is less experienced with reviews. Presumably, anyone in the triager role would have a reputation for leaving useful feedback.
Restricting the label changing to the triager role would reduce the net number of awaiting core review labels, but it would help to make the label more meaningful for the core devs. If an unknown contributor approves a PR without leaving any commentary and the label changes to awaiting core review, it doesn’t do much too help the workflow. At least in my opinion.
Edit: As an alternative, perhaps the awaiting core review label would be automatically added after a triager left a review of any form, but not from a contributor approving or suggesting changes. The main purpose of my suggestion was just to remove the issue of the label being added when contributors leave an approval review without any commentary.
Previously, there was another discussion involving a similar topic: Disable approving reviews of external contributors - #17 by aeros. This would likely fix the issue that this discussion addressed, but without affecting the ability for contributors to approve reviews.
I don’t like this idea, it gives the impression that we don’t care at all about reviews from non-triagers. The status-quo is already very conservative, I don’t see anything wrong with it.
If you really want to make a difference between triagers and non-triagers, I would instead suggest to treat reviews from triagers the same way as reviews from core devs (i.e. status set to awaiting changes/awaiting merge).
This might be a decent alternative. My intention is not at all to devalue reviews from non-triagers, but instead to improve the potential positive impact that triagers would have upon the workflow. Using the awaiting merge and awaiting changes labels would also work, and potentially lead to less confusion to those who are used to the current behavior of awaiting core review.
However, that might conflict with this to some degree:
Unless I’m mistaken, the awaiting changes label prompts a direct message from the bot for the author to modify their PR, which might come across a bit too strongly. So perhaps that behavior could be shifted to a new label such as awaiting core changes or something similar, which would only be used when a core dev requests changes.
The awaiting merge label should be okay to apply if a triager approves the PR, as that label does not have any direct functionality as far as I’m aware.
This only triggers if a core dev requests changes, so it’s meant to be strong.
The point isn’t triggering functionality, it’s meant to signal to core developers that the PR is ready to be merged and thus further review isn’t necessary. So it shouldn’t be done by triagers as that would be bypassing core devs for review.