-
Notifications
You must be signed in to change notification settings - Fork 13k
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: Notify Zulip for rustdoc nominated issues #82689
Conversation
This was a good idea :) |
@camelid feel free to r=me once you've figured out what to do about your comments on the PR. |
The rustdoc team does not currently use the `I-nominated` label, unlike the libs and compiler teams (and maybe others). One reason for this is that the other teams discuss their nominated issues in meetings, while rustdoc is an async-only team. However, it might be helpful to start using the `I-nominated` label for rustdoc. The team currently uses a `cc @rust-lang/rustdoc` ping as the equivalent, but it's easier to track issues when they use `I-nominated`. Also we'd be more consistent with the other teams' procedures. Since rustdoc doesn't have meetings, I propose we instead use the triagebot notify Zulip functionality to create a topic in `#rustdoc` on Zulip and ping the team. So it would look a bit like the procedure for WG-prioritization when an issue acquires the `I-prioritize` label.
c07ad7c
to
59ee872
Compare
(Squashed.) |
[notify-zulip."T-rustdoc"] | ||
required_labels = ["I-nominated"] | ||
zulip_stream = 266220 # #rustdoc | ||
topic = "nominated: #{number}" | ||
message_on_add = """\ | ||
@*T-rustdoc* issue #{number} "{title}" has been nominated for `T-rustdoc` discussion. | ||
""" | ||
message_on_remove = "Issue #{number}'s nomination request has been removed." |
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.
The one issue with this setup is that we will only be notified if T-rustdoc
is removed—nothing will happen if I-nominated
is removed. It might be worth getting rid of the message_on_remove
since it will be kind of confusing.
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.
Does an array work here? cc @Mark-Simulacrum can we do [[notify-zulip."I-nominated"]]
here?
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.
No, I don't think so, but I also don't really know what you mean by that. That said, it seems quite reasonable for triagebot to detect the removal of a required label and treat that just like removal of T-rustdoc; I think the reasonable treatment is that the whole set of labels must be present and removal of any leads to a notification of such. Happy to accept a PR to that effect.
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.
That said, it seems quite reasonable for triagebot to detect the removal of a required label and treat that just like removal of T-rustdoc; I think the reasonable treatment is that the whole set of labels must be present and removal of any leads to a notification of such. Happy to accept a PR to that effect.
That might be a natural extension of the work in rust-lang/triagebot#1078—Unlabeled
would be treated the same as Closed
. Probably not worth blocking this PR on that one though.
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.
No, I don't think so, but I also don't really know what you mean by that.
I think he means that we would be notified as soon as both I-nominated
and T-rustdoc
were present, and we would be notified again as soon as one of them was removed. Treating the removal of a required label the same as the removal of the main label would have the same effect, but gives the labels an asymmetric relationship (T-rustdoc
is considered the "main" label by triagebot).
I think I'll wait until one or two more people approve, and for feedback on #82689 (comment), before I merge. |
Broadly looks good to me, but these things are finicky, so I recommend merging and trying it out. |
@bors r=Manishearth |
📌 Commit 59ee872 has been approved by |
Rollup of 10 pull requests Successful merges: - rust-lang#81223 ([rustdoc] Generate redirect map file) - rust-lang#82439 (BTree: fix untrue safety) - rust-lang#82469 (Use a crate to produce rustdoc tree comparisons instead of the `diff` command) - rust-lang#82589 (unix: Non-mutable bufs in send_vectored_with_ancillary_to) - rust-lang#82689 (meta: Notify Zulip for rustdoc nominated issues) - rust-lang#82695 (Revert non-power-of-two vector restriction) - rust-lang#82706 (use outer_expn_data() instead of outer_expn().expn_data()) - rust-lang#82710 (FloatToInit: Replacing round_unchecked_to --> to_int_unchecked) - rust-lang#82712 (Remove unnecessary conditional `cfg(target_os)` for `redox` and `vxworks`) - rust-lang#82713 (Update cargo) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
The rustdoc team does not currently use the
I-nominated
label, unlikethe libs and compiler teams (and maybe others). One reason for this is
that the other teams discuss their nominated issues in meetings, while
rustdoc is an async-only team.
However, it might be helpful to start using the
I-nominated
label forrustdoc. The team currently uses a
cc @rust-lang/rustdoc
ping as theequivalent, but it's easier to track issues when they use
I-nominated
.Also we'd be more consistent with the other teams' procedures.
Since rustdoc doesn't have meetings, I propose we instead use the
triagebot notify Zulip functionality to create a topic in
#rustdoc
onZulip and ping the team. So it would look a bit like the procedure for
WG-prioritization when an issue acquires the
I-prioritize
label.cc @rust-lang/rustdoc for approval of the change
r? @Mark-Simulacrum to make sure I configured triagebot correctly