Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create Needs PR and Needs decision labels #6999

Closed
DanielNoord opened this issue Jun 22, 2022 · 9 comments 路 Fixed by #7004
Closed

Create Needs PR and Needs decision labels #6999

DanielNoord opened this issue Jun 22, 2022 · 9 comments 路 Fixed by #7004

Comments

@DanielNoord
Copy link
Collaborator

I have followed this discussion over at CPython Discourse and was triggered by:
being able to tell at a glance based on a label why an issue is still open is really powerful (hence the needs labels on PRs 馃榿)

I think this is also very true for our own project and some of the labels we use.
As you probably noticed yesterday I tweaked some of the colours on our labels to clearly indicate no action is needed from a maintainer. Labels like work in progress, waiting on author and needs astroid update now share the same colour so you know as a maintainer that such PRs don't require your attention. Similarly, needs triage and needs review are now bright yellow and stand out so as a maintainer your attention is immediately drawn to them.
For PRs I think this already improves the situation somewhat although further improvement are obviously possible.

For issues I'd like to also improve our triaging a bit. That's why I would introduce new Needs PR and Needs decision labels. We wouldn't need to label all existing issue, but can just use them as we encounter relevant cases.
Needs PR would cover all cases where there is explicit approval or clear reason for implementation, such as with bug fixes. Notice that for new messages I would say that deciding on a message name would be a prerequisite for the label.
Needs decision would cover all issues where a maintainer needs to make a decision, such as approving a message name.

We can note some of the labels down in our contributing guidelines and hopefully such explicit labels would help reduce the "has this been fixed yet?" comments.

@Pierre-Sassoulas
Copy link
Member

Agree, it makes a lot of sense. It's very nice to be able to indicate what needs to be done with the label change (if you don't have the time to really triage but the issue seems "probably reasonable", you just add "needs investigation" as in 'it's not utter garbage but still needs some investigation' so your partial work is not lost)

I was also thinking recently that we should review "proposal" and "discussion" at release time. I often add discussion to milestone so a decision is taken by then but a "needs decision" label makes more sense and is a lot faster to do. How about we review them all when we release a minor too ?

Btw slightly related but there's an issue with label suddenly showing :emoticon:"> in their short description, do you have an idea why ?

labelissue

@DanielNoord
Copy link
Collaborator Author

Agree, it makes a lot of sense. It's very nice to be able to indicate what needs to be done with the label change (if you don't have the time to really triage but the issue seems "probably reasonable", you just add "needs investigation" as in 'it's not utter garbage but still needs some investigation' so your partial work is not lost)

  1. Needs investigation: maintainer needs to check if the code is indeed causing a bug.
  2. Needs decision: maintainer needs to decide if the proposal/question is worth implementing
  3. Needs PR: maintainer has approved the proposal, question or bug report and a PR is welcome
  4. Needs triage: the issue needs the appropriate labels and has not been seen by a maintainer yet

I would reserve investigation for bug fixes and inspection of code/crashes.

1, 2 and 4 would have the same colour as they require maintainer action. 3 should probably be the same colour as Help wanted.

I was also thinking recently that we should review "proposal" and "discussion" at release time.

That would be good but there are some discussion and proposals that have been open for a couple of years because they are difficult to get right. I think discussion and proposal are a different stage than needs decision, but we're getting into semantics. For me discussion and proposal signal that there is ongoing exchange of arguments, whereas needs decision signals that the discussion has been exhausted and a maintainer just needs to say "we're going to do this and that".
So discussion and proposal don't necessarily resolve themselves in one release cycle.

For example, I don't mind keeping #5701 open a little longer. We won't implement it in 2.15 anyway so there is no rush. #5788 and #5940 on the other hand seem fairly straightforward and just need a final comment from a maintainer on what the next course of action is.

Btw slightly related but there's an issue with label suddenly showing :emoticon:"> in their short description, do you have an idea why ?

I think Github might be testing searching on emojis in the label tab? Not sure though.

@Pierre-Sassoulas
Copy link
Member

I would reserve investigation for bug fixes and inspection of code/crashes.

Sure most of the time if you can't triage in less than 5mn it's because it's a complicated bug/crash.

Maybe we could also add 'Need reproduction' for acknowledged but not reproduced yet. For crashes I often trust the reporter but when the issue is on old versions it's a good idea to reproduce on main first.

So discussion and proposal don't necessarily resolve themselves in one release cycle.

we should review "proposal" and "discussion" at release time was an old idea I did not open an issue for because of this, 'need decision' resolves the issues with old discussion that are not easy to conclude perfectly 馃憣

@DanielNoord
Copy link
Collaborator Author

@Pierre-Sassoulas I have edited the labels slightly. Now all labels which require maintainer action have the colour #ffff00 while non-action (such as waiting on author) is #5319E7.

@Pierre-Sassoulas
Copy link
Member

Would you mind if I change to light pink or something ? This yellow is really flashy/agressive 馃槃

@DanielNoord
Copy link
Collaborator Author

Would you mind if I change to light pink or something ? This yellow is really flashy/agressive 馃槃

Well I'm quite colour blind, so I probably don't notice it as much as you do. 馃槃 I liked the blue/purple/whatever of #5319E7 as for me it is clearly separated from the "vague" colours of the topic-* labels and I can train myself to disregard all PRs with that colour.
What about #ffa500?

@Pierre-Sassoulas
Copy link
Member

Hmm if you're color blind it's probably better if you choose the color yourself, I can live with flashy color easier than you can live with color that look the same to you. #ffa500 is nicer but probably very close to the astroid's label.

@DanielNoord
Copy link
Collaborator Author

DanielNoord commented Jun 22, 2022

Hmm if you're color blind it's probably better if you choose the color yourself, I can live with flashy color easier than you can live with color that look the same to you. #ffa500 is nicer but probably very close to the astroid's label.

Let's use #ffa500 and give the astroid labels something different (I'll leave the choice to your better performing eyes 馃槃). For me the main benefit of the colours is to quickly grab my attention for issues/PRs where I can be of help. You can easily ignore all colours of all other labels but over time as a maintainer/contributor you'll get used to needing to respond to #ffa500. I think that's a nice benefit!

@Pierre-Sassoulas
Copy link
Member

I changed some color following your instruction and removed "task" in favor of "maintenance".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants