Update the triage/triaging.rst page. #914
Conversation
|
This is a nice improvement. IMO, when issues are closed, we should also encourage leaving a message regarding why it was closed, for example "implemented in the following PRs: ", "superseded by ", etc. |
|
Also, I made a review checklist for one of my PRs: python/cpython#93823 (comment) Perhaps a generalised version could be added to the devguide? |
|
I added a mention of the
SGTM -- that's a nice list. I think it would be easier to add it in a separate PR though.
The former has been likely written from scratch after the HG->Git migration, whereas the latter formerly documented how to review patches attached to a bpo issue and then it has been adapted to GitHub. I think the best approach would be to enhance the |
SGTM! Thanks for making all these enhancements to our (documented) workflow. |
I created #962 to track this, so that we can start merging this and keep improving the devguide iteratively (as Diataxis suggests). |
triage/triaging.rst
Outdated
| * You might also leave a brief comment about the proposed next action needed. | ||
| If there is a long message list, a summary can be very helpful. | ||
| 8000 | * Finally, you can set the :gh-label:`triaged` label (unless you want other triagers | |
| to take a look). |
There was a problem hiding this comment.
This is so far the first and only documentation we have about the "triaged" label, so let me try to understand what it's for. The idea is that if a triager does not want other triagers to look at the issue, they put a triaged label?
Why would a triager not want other triagers to look at the issue?
Is it then offensive for another triager or core dev to change the labels on the issue (undermining a decision that another triager marked as 'final')?
This sounds to me like the opposite of how we work. It's the equivalent of a label that a no-committer can put on a PR to say "this has been reviewed. I do not want other non-commiters to review it". We don't have this label. We have the "awaiting core review" label which indicates that a non-committer has reviewed, but it does not dis-invite further reviews from non-committers.
There was a problem hiding this comment.
I see now that if I hover over the label it says "The issue has been accepted as valid by a triager". Which is quite different from what is implied by this doc.
I don't think validity is something we expect triagers to decide about (when I proposed a "valid" label I tried to be clear that it is to be used by subject matter experts, not triagers). If you think that this is for triagers to decide, then this doc should explain how a triager is supposed to make that decision. If you don't think that, then the tooltip on the "triaged" label needs to change to whatever it is that this label means.
There was a problem hiding this comment.
I don't remember the exact timeline, but I wrote that sentence on Aug 6 and I might have added the label description on GitHub later after more discussions.
However it seems there is still no clear consensus on how it should be used. Are you at the sprints? Perhaps it could be discussed there.
Regarding this specific PR, it is mostly about updating the checklist, not documenting the label itself. I can remove or change the sentence, and possibly re-add/update it again in next iteration. I also have another incoming PR (#930) where I can document it more extensively.
There was a problem hiding this comment.
What meaning did the Steering Council intend when they approved the new term?
I agree with deferring the Triaged sentence to get on with the improved list.
There was a problem hiding this comment.
See https://discuss.python.org/t/improve-communication-with-contributors-on-the-issue-tracker-and-prs-language-summit-follow-up/15812/30, in particular:
The triaged label would signify that the process is done (for now, at least) – and since the issue wasn’t closed, it’s “valid” and waiting for a core dev.
There was a problem hiding this comment.
Are you at the sprints? Perhaps it could be discussed there.
I'm online, but I don't feel able to contribute on this matter beyond commenting on whether I think the documentation is consistent and clear.
There was a problem hiding this comment.
The idea is that if a triager does not want other triagers to look at the issue, they put a triaged label?
No, the idea is that if a triager thinks there is nothing more a triager can do, they mark the issue so others don't need to re-read it.
Is it then offensive for another triager or core dev to change the labels on the issue (undermining a decision that another triager marked as 'final')?
That would be like any other disagreement. It could be done in an offensive way, but hopefully won't.
this has been reviewed. I do not want other non-commiters to review it
I'd word it as “This has been reviewed. Other non-commiters are free to re-review it, but I believe their time would be better spent elsewhere.”
(I don't speak for the entire Steering Council here.)
There was a problem hiding this comment.
The triaged label would signify that the process is done (for now, at least)
Does “for now, at least” mean that the triaged label will be removed by a bot after a certain amount of time? Or how else will it work long term?
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
This PR updates the
triage/triaging.rstpage, which contained outdated references to bpo and HG's patch-based workflow.See https://cpython-devguide--914.org.readthedocs.build/triage/triaging/