Who should be able to change what labels on GitHub?

[Mariatta]

@aeros

Also, it might be worth taking the time to make the labels across the Python repos as universal as possible. This might be going outside of the scope of the triager role specifically, but I figured it was relevant since we’re going to be adding a GitHub labels section as part of updating the devguide

This is out of scope for triagers but a bot is being developed between myself and Raphael, to manage labels among GitHub repositories.

@aeros

DO-NOT-MERGE , expert-asyncio , invalid , OS-mac , OS-windows , skip issue , skip news , sprint , type-bugfix , type-documentation , type-enhancement , type-performance , type-security , type-tests

From that list, the ones I’m uncertain about for triagers would be DO-NOT-MERGE , invalid , (due to them coming across strongly) and potentially type-security . The distinction between a PR being type-bugfix vs type-security might not be immediately clear. As an example, I initially had assumed that this PR would have been considered a security issue instead of a bugfix due to it being able to crash the interpreter with a segfault.

One guideline is, when in doubt, ask first before adding/removing label, and don’t remove labels added by core devs.

Perhaps this need to be split to a new thread but I don’t have the ability to do that, perhaps one of the admins will do it

invalid: This can be automatically added by bedevere when people try to merge maintenance branch to the master branch. It can be manually added by us too if we deem it invalid. The effect is, the PR won;t count towards user’s contribution during hacktoberfest.

DO-NOT-MERGE: a core dev would usually apply this label. miss-islington knows not to merge PRs with this label. I think it should be ok for triagers to add the “do-not-merge” label when necessary, but let’s not remove it if core dev has added the label. Hope this makes sense?

needs backport to X.Y: you’re right that it will trigger miss-islington to autobackport, however, miss-islington won’t be able to merge the backport PR unless approved by a core dev. I’m thinking that it is harmless for a triager to trigger the autobackport, if anything it can help speed up the process. But I’m open to be convinced otherwise on this.

sprint: added when the PR was created during in person sprint (e.g. at PyCons, at EuroPython, at core sprint, etc). The idea is to prioritize reviewing them during the sprint.

type-documentation, type-tests: auto added by bedevere when the PR contains only doc changes (rst) or only tests changes. People interested in reviewing docs or tests-related PRs can use the label to find PRs to review.

expert-asyncio, OS-mac, OS-windows, other type-...: They’re manually added. No effect to bots. Should be fine for triagers to add these labels to help categorize PRs accordingly. And of course, we trust triagers not to abuse their ability.

[aeros (Kyle Stanley)]

Thanks for the clarifications. There’s quite a number of labels to go through, and I wanted to make sure I didn’t misunderstand any of them.

@Mariatta

type-documentation , type-tests : auto added by bedevere when the PR contains only doc changes (rst) or only tests changes. People interested in reviewing docs or tests-related PRs can use the label to find PRs to review.

So, to go a little bit more in-depth with the type labels, is it possible for a PR to have multiple type labels when appropriate or should it be restricted to one?

Also, how do the core devs currently differentiate between an issue being considered a type-bugfix vs type-security? At least to me, the distinction seems like it could be a bit subjective. Part of it probably relies on experience, but I’m just looking for general guidelines to add to the devguide triagers section.

[Mariatta]

@aeros

So, to go a little bit more in-depth with the type labels, is it possible for a PR to have multiple type labels when appropriate or should it be restricted to one?

It should be fine to have multiple type-... labels in one PR.

@aeros

how do the core devs currently differentiate between an issue being considered a type-bugfix vs type-security

For security, if it is related to the “security” type issue in the bpo ticket, then the PR should likely be tagged with type-security too. There’s existing section describing the security type:

Issues that might have security implications. Report security vulnerabilities using the procedure found in the Reporting security issues in Python page on the python.org website.

For bug fix, this is where it might become a subjective opinion, and having more experience with CPython workflow would help. To me, it is more about distinguishing if the change is new feature or not. If it is not a new feature, then likely it is a bug fix, and likely needs to be backported. To me, bug fixes could also mean when things are not behaving in the way they’re documented. Also important to know that just because the code exists in maintenance branch, it does not mean that it must be backported. Not sure if this is clear?

[aeros (Kyle Stanley)]

@Mariatta

For bug fix, this is where it might become a subjective opinion, and having more experience with CPython workflow would help. To me, it is more about distinguishing if the change is new feature or not. If it is not a new feature, then likely it is a bug fix, and likely needs to be backported. To me, bug fixes could also mean when things are not behaving in the way they’re documented. Also important to know that just because the code exists in maintenance branch, it does not mean that it must be backported. Not sure if this is clear?

Yes, this definitely helps to clear things up for me. As you said, I think a decent amount of it will come down to having further experience with the workflow. But, the summaries I’ve received from you and the other core devs have definitely helped clear things up a good amount, at least enough to get started.

At this point, I feel comfortable enough with the labels to start working on a draft for a GitHub labels section in the traigers.rst page of the devguide. Thanks again for all of the helpful insight.

[aeros (Kyle Stanley)]

With regards to the type-documentation label, would it also be appropriate for PRs which modify code comments or docstrings without changing the code itself? I recently thought of this while working on the PR to add the labels to the devguide.

[brettcannon]

@aeros

With regards to the type-documentation label, would it also be appropriate for PRs which modify code comments or docstrings without changing the code itself?

Good question and I don’t think we have an answer. I guess it makes sense.

[aeros (Kyle Stanley)]

@brettcannon

Good question and I don’t think we have an answer. I guess it makes sense.

Thanks for the clarification, I’ll add it to the devguide GitHub label PR then. It’s currently still in a draft stage though, I was planning on mentioning you, Eric Snow, and Carol in the PR once all of the CPython labels are included for feedback (Mariatta left some earlier).

I have a feeling that some of them could probably use an improvement to their descriptions. This was just a first attempt based on questions I’ve asked various core devs since there’s not much existing documentation that mentions the labels. I’m hoping that it will become a useful resource for anyone on the Python triage team and any contributors that want to learn more about the GitHub labels.