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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[meta] Clarify prioritization alert #119959

Merged
merged 1 commit into from
May 13, 2024

Conversation

fmease
Copy link
Member

@fmease fmease commented Jan 14, 2024

Apparently, there used to exist the label I-nominated judging from this entry:

required_labels = ["I-nominated"]

Since it was replaced with individual team labels, I think it makes sense to update the prioritization alert. Of course, it's not super important since the members of WG-prioritization already know that. This is just cleanup.

r? apiraino or wg-prioritization

@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 14, 2024
@fmease fmease added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Jan 14, 2024
@apiraino
Copy link
Contributor

@fmease ah yes well spot 👍

Now that you mention that label, I grepped through other git repos (triagebot, rustforge and compiler-team) and there are other mentions of that legacy label. I will make a note to take care of them.

@@ -383,7 +383,7 @@ message_on_add = """\
- Priority?
- Regression?
- Notify people/groups?
- Needs `I-nominated`?
- Needs `I-{team}-nominated`?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious how the {team} placeholder is resolved (probably by the triagebot?) 🤔

I'll check that, too.

Copy link
Member Author

@fmease fmease Jan 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, {variable} gets substituted by triagebot, I forgot. Well, I don't know how triagebot will deal with this, I don't think it will substitute a team since team isn't defined anywhere I assume. {team} was actually only meant as a placeholder for humans (“some team”) not for triagebot. If you have concrete suggestions I can change the placeholder.

Copy link
Contributor

@apiraino apiraino Jan 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think when the Triagebot is about to emit the notification to Zulip (in this point) if there is a T-* label associated with the issue I can infer which team to use to replace {team}.

Not 100% sure it works as I suspect, I need to check a few assumptions but worth a try. Let me get back to you on this before merging this patch 🙂

Copy link
Member Author

@fmease fmease Jan 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can submit a patch to triagebot, too, if you'd like to. It should be as simple as:

let mut teams = event
    .issue
    .labels
    .iter()
    .map(|label| &label.name)
    .filter_map(|label| label.strip_prefix("T-"));
if let Some(team) = teams.next()
    && teams.next().is_none()
{
    topic = topic.replace("{team}", team);
}

However, it would hard-code the concept of “teams” and rust-lang/rust's labeling system (in which T-* labels represent teams) and as far as I remember, triagebot wants to stay repository-agnostic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fmease left a proposal at rust-lang/triagebot#1766

@fmease
Copy link
Member Author

fmease commented Jan 29, 2024

How should we proceed?

@apiraino
Copy link
Contributor

How should we proceed?

apologies @fmease I lost track of this one. I'll prioritize to check your suggestion.

@fmease fmease added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Feb 3, 2024
@Dylan-DPC Dylan-DPC removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 21, 2024
@apiraino apiraino assigned apiraino and unassigned apiraino Apr 16, 2024
@apiraino
Copy link
Contributor

r? @apiraino

@apiraino
Copy link
Contributor

ok rust-lang/triagebot#1766 has been merged so this is now unblocked.

Let's merge this and break the notification workflow :)

r=me

@rustbot label -S-blocked

@rustbot rustbot removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label May 13, 2024
@fmease
Copy link
Member Author

fmease commented May 13, 2024

@bors r=apiraino rollup

@bors
Copy link
Contributor

bors commented May 13, 2024

📌 Commit 4333fb0 has been approved by apiraino

It is now in the queue for this repository.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 13, 2024
fmease added a commit to fmease/rust that referenced this pull request May 13, 2024
…ert-label, r=apiraino

[meta] Clarify prioritization alert

Apparently, there used to exist the label <kbd>I-nominated</kbd> judging from this entry:

https://github.com/rust-lang/rust/blob/8847bda592d940ae1f34d87d8cacdc09a9f787fa/triagebot.toml#L393

Since it was replaced with individual team labels, I think it makes sense to update the prioritization alert. Of course, it's not super important since the members of WG-prioritization already know that. This is just cleanup.

r? apiraino or wg-prioritization
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 13, 2024
…ert-label, r=apiraino

[meta] Clarify prioritization alert

Apparently, there used to exist the label <kbd>I-nominated</kbd> judging from this entry:

https://github.com/rust-lang/rust/blob/8847bda592d940ae1f34d87d8cacdc09a9f787fa/triagebot.toml#L393

Since it was replaced with individual team labels, I think it makes sense to update the prioritization alert. Of course, it's not super important since the members of WG-prioritization already know that. This is just cleanup.

r? apiraino or wg-prioritization
bors added a commit to rust-lang-ci/rust that referenced this pull request May 13, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#119515 (style-guide: Format single associated type `where` clauses on the same line)
 - rust-lang#119959 ([meta] Clarify prioritization alert)
 - rust-lang#123817 (Stabilize `seek_seek_relative`)
 - rust-lang#124532 (elaborate obligations in coherence)
 - rust-lang#125063 (Don't call `env::set_var` in `rustc_driver::install_ice_hook`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request May 13, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#119515 (style-guide: Format single associated type `where` clauses on the same line)
 - rust-lang#119959 ([meta] Clarify prioritization alert)
 - rust-lang#123817 (Stabilize `seek_seek_relative`)
 - rust-lang#125063 (Don't call `env::set_var` in `rustc_driver::install_ice_hook`)
 - rust-lang#125071 (Migrate rustdoc target spec json path)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit cf8a084 into rust-lang:master May 13, 2024
11 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 13, 2024
@fmease fmease deleted the triagebot-prioritization-alert-label branch May 13, 2024 22:21
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 13, 2024
Rollup merge of rust-lang#119959 - fmease:triagebot-prioritization-alert-label, r=apiraino

[meta] Clarify prioritization alert

Apparently, there used to exist the label <kbd>I-nominated</kbd> judging from this entry:

https://github.com/rust-lang/rust/blob/8847bda592d940ae1f34d87d8cacdc09a9f787fa/triagebot.toml#L393

Since it was replaced with individual team labels, I think it makes sense to update the prioritization alert. Of course, it's not super important since the members of WG-prioritization already know that. This is just cleanup.

r? apiraino or wg-prioritization
@apiraino apiraino mentioned this pull request May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself C-cleanup Category: PRs that clean code up or issues documenting cleanup. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants