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.

Sign up for GitHub

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

Jump to bottom

meta: Notify Zulip for rustdoc nominated issues #82689

Merged
merged 1 commit into from
Mar 3, 2021
Merged

meta: Notify Zulip for rustdoc nominated issues #82689

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions triagebot.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,15 @@ message_on_add = """\
"""
message_on_remove = "Issue #{number}'s prioritization request has been removed."

[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."
Comment on lines +105 to +112
Copy link
Member Author

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.

Copy link
Member

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?

Copy link
Member

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.

Copy link
Member Author

@camelid camelid Mar 2, 2021 •
edited
Loading

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.

Copy link
Member Author

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).


[github-releases]
format = "rustc"
project-name = "Rust"
Expand Down