Do not send a notification for P-high stable regressions #73585
Conversation
rust-highfive
added
the
S-waiting-on-review
triagebot.toml
Outdated
| @@ -113,7 +113,7 @@ topic = "P-critical #{number} {title}" | |||
| message_on_add = "@*WG-prioritization* issue #{number} has been assigned `P-critical`." | |||
|
|
|||
| [notify-zulip."P-high"] | |||
| required_labels = ["regression-from-stable-to-*"] | |||
| required_labels = ["regression-from-stable-to-[bn]*"] | |||
There was a problem hiding this comment.
I would prefer that we just list out beta and nightly explicitly (I forget if this is a regex or what, too, it might be using shell-like globs in which case this won't work).
There was a problem hiding this comment.
Yes, we are currently using globs for this.
There was a problem hiding this comment.
Ah okay, seems like globset does support this notation. I would still prefer two entries in this array.
There was a problem hiding this comment.
This means I'll have to modify the behaviour of Triagebot. I implemented it in an "AND" fashion, so that all the labels in the array are required. So, if we think we'll never have to combine multiple labels such that all of them are required, I can just open a PR in Triagebot to switch that. Otherwise we'll have to find another solution. What do you think? cc @spastorino
There was a problem hiding this comment.
Ah okay, makes sense. I think that's probably good to leave in. Mind changing this to use -{beta, nightly} presuming that's supported?
There was a problem hiding this comment.
I've read the documentation of glob and I don't think it is supported. I will check again though.
There was a problem hiding this comment.
I don't think it is supported. See the glob documentation and the test I just did in the playground.
There was a problem hiding this comment.
Ah, okay. Maybe we should switch to globset which seems more featureful (or just go to regex directly).
|
r=me with commits squashed presuming consensus on wanting the change itself. |
Add comment to clarify the pattern
|
This was asked by @spastorino, see this message. @Mark-Simulacrum just squashed the commits, thanks for reviewing :) |
|
@bors r+ rollup Thank you! |
|
📌 Commit ae71e96 has been approved by |
bors
added
S-waiting-on-bors
…ulacrum Do not send a notification for P-high stable regressions This is kind of a hack to only match nightly and beta regressions, but not stable regressions. See my tests [on the playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=6ff8a809162118aa2951f2ff12400067). r? @spastorino cc @Mark-Simulacrum
…ulacrum Do not send a notification for P-high stable regressions This is kind of a hack to only match nightly and beta regressions, but not stable regressions. See my tests [on the playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=6ff8a809162118aa2951f2ff12400067). r? @spastorino cc @Mark-Simulacrum
…ulacrum Do not send a notification for P-high stable regressions This is kind of a hack to only match nightly and beta regressions, but not stable regressions. See my tests [on the playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=6ff8a809162118aa2951f2ff12400067). r? @spastorino cc @Mark-Simulacrum
Rollup of 6 pull requests Successful merges: - rust-lang#71660 (impl PartialEq<Vec<B>> for &[A], &mut [A]) - rust-lang#72623 (Prefer accessible paths in 'use' suggestions) - rust-lang#73502 (Add E0765) - rust-lang#73580 (deprecate wrapping_offset_from) - rust-lang#73582 (Miri: replace many bug! by span_bug!) - rust-lang#73585 (Do not send a notification for P-high stable regressions) Failed merges: - rust-lang#73581 (Create 0766 error code) r? @ghost
This is kind of a hack to only match nightly and beta regressions, but not stable regressions. See my tests on the playground.
r? @spastorino cc @Mark-Simulacrum