-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs(contrib): Improve triage instructions #14052
Conversation
r? @weihanglo rustbot has assigned @weihanglo. Use |
@@ -225,9 +228,9 @@ There are several things to consider when triaging an issue: | |||
needs to discuss whether or not to proceed and what needs to be done to | |||
address the issue. | |||
* [S-needs-mentor] --- This is something the Cargo team wants to address, | |||
but does not currently have the capacity to help with reviewing. | |||
but does not currently have the capacity to help with reviewing. **(reserved for Cargo team)** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about S-needs-rfc
?
Anyone is welcome to help at this stage, but it should be clear that it is not yet accepted. However, this should only be tagged for changes that are somewhat likely to be accepted.
Should we include that or adjust its text a bit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took out the "likely to be accepted". I didn't realize that and have been tagging things that are RFC scope. I did link out to the instructions on socializing an RFC which kind of fills a similar role of pre-vetting.
src/doc/contrib/src/issues.md
Outdated
@@ -217,17 +221,17 @@ There are several things to consider when triaging an issue: | |||
* Assuming the issue looks valid, remove the [S-triage] label and move it onto |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the [S-triage] label
This is part of what doesn't sit well with me. What if the next natural status is one of the (reserved for Cargo team) ones? Isn't removing [S-triage] reserved as well then?
I suggest something like this:
Assuming the issue looks valid, consider assigning it a new status label:
... (bullet points)
Remove the [S-triage] label if you moved the issue to a new status.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried a slightly different wording
Assuming the issue looks valid, switch the [S-triage] label for one of the following:
My thinking is that this makes it clearer that this is an "atomic" operation; that you should only remove S-triage
if you can add the new label. In practice, if there is no more design work, then its "needs-team-input".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change looks reasonable. Thanks!
@bors r+ |
☀️ Test successful - checks-actions |
Update cargo 13 commits in 4dcbca118ab7f9ffac4728004c983754bc6a04ff..a1f47ec3f7cd076986f1bfcd7061f2e8cb1a726e 2024-06-11 16:27:02 +0000 to 2024-06-15 01:10:07 +0000 - Change verification order during packaging. (rust-lang/cargo#14074) - Update git2 for libgit2 1.8.1 (rust-lang/cargo#14067) - Fix some documentation misspellings (rust-lang/cargo#14066) - chore(deps): update msrv (1 version) to v1.79 (rust-lang/cargo#14063) - test: Redact conditional compile-fail warning (rust-lang/cargo#14064) - Migrate a few test files to snapbox (rust-lang/cargo#14048) - docs(contrib): Improve triage instructions (rust-lang/cargo#14052) - chore(ci): Upgrade cargo-semver-checks (rust-lang/cargo#14062) - Revert rust-lang/cargo#13630 as rustc ignores `-C strip` on MSVC (rust-lang/cargo#14061) - test: migrate features_are_quoted to snapbox (rust-lang/cargo#14051) - Add assert redactions (rust-lang/cargo#14054) - test: migrate build_script_env to snapbox (rust-lang/cargo#14044) - docs: Iterate on --breaking docs (rust-lang/cargo#14047) r? ghost
What does this PR try to resolve?
@torhovland brought up some confusions they had when looking at our triage instructions and this attempts to fix them.
How should we test and review this PR?
Additional information