rust-lang / triagebot

Automation/tooling for Rust spaces
https://triage.rust-lang.org
Apache License 2.0
170 stars 74 forks source link

Add options to silence mentions message when a particular assignment is used #1664

Closed WaffleLapkin closed 7 months ago

WaffleLapkin commented 1 year ago

I'm not sure if this is the right way to do it, but I think this should work.

I want to remove these long comments about libs-api team, when r? libs-api is used:

2022-10-29_00-05

The idea is to add

[assign.label]
libs-api = ["T-libs-api"]

To the Rust triagebot.toml, which will add T-libs-api label when r? libs-api is used. Then T-libs won't be added because there is already a T-* label.

Then, to silence the message add

# [mentions."library"]
exclude_labels = ["T-libs-api"]

Again, the "solution" and "implementation" seem somewhat hacky. I'm open to suggestions how to better do this.

Mark-Simulacrum commented 1 year ago

I'm not sure I fully understand the motivation here. It seems to me like the message is generally not a big deal? It also seems like hiding it based on something obscure like picking a particular way to roll the reviewer isn't all that appealing to me.

I think I could be convinced that we should make the auto-comment message sometimes conditional on whether we consider a contributor "new" - but even there, it feels like there's a pretty high likelihood it won't trigger when we do want it to.

Overall I think my annoyance I guess with it being posted is pretty low on any given PR, so that makes it very hard to justify custom logic here.

WaffleLapkin commented 1 year ago

Maybe this is just a "me problem", which is good, cause we don't need to do anything then :)

It also seems like hiding it based on something obscure like picking a particular way to roll the reviewer isn't all that appealing to me.

I don't think it's obscure, if r? libs-api is used, then we know that this is a PR for T-libs-api, not T-libs, so we can apply the right label, at which point there is no reason to show the message.

I'm mostly annoyed that there is no way to make a T-libs-api PR from the beginning. IIRC @rustbot label -T-libs +T-libs-api doesn't work in the PR description, the labels after that are wrong (don't recall in which way though).

I think I could be convinced that we should make the auto-comment message sometimes conditional on whether we consider a contributor "new" - but even there, it feels like there's a pretty high likelihood it won't trigger when we do want it to.

I actually don't think it's a good idea. Even if the contributor is not new (how new?) they could forget about libs/-api difference or it may be the first library PR, even though they contributed to the compiler a lot...

Mark-Simulacrum commented 1 year ago

Hiding specifically libs-api message because you've explicitly selected libs-api does seem okay, but many of our other messages actively ping people - which we probably don't want to silence with a label.

I think in general this kind of thing is a bit hard with the current architecture, as mentions and reviewer assignment happen in different steps and ordering also isn't guaranteed (or whether they'll see a label set by a previous step).

I think the general desire to make things quieter when they're likely not helpful is a good one, but I don't think this is quite the right approach (not that I necessarily have better suggestions though).

WaffleLapkin commented 7 months ago

I don't need/want this anymore, and it doesn't looks like anyone else does. Thus — closing.