feat(notify-zulip): support notify-zulip.<label> getting mapped to multiple actions

[ismailarilik]

This is a requested feature in the Rust project: https://github.com/rust-lang/rust/blob/master/triagebot.toml#L492 (Notice FIXME comments there)

With this PR, multiple notify-zulip tables are possible. For example:

[notify-zulip."beta-nominated"]
required_labels = ["T-rustdoc"]
zulip_stream = 266220
topic = "beta-nominated: #{number}"
message_on_add = "PR #{number}'s beta-nomination has been added."
message_on_remove = "PR #{number}'s beta-nomination has been removed."
message_on_close = "PR #{number} has been closed. Thanks for participating!"
message_on_reopen = "PR #{number} has been reopened. Pinging @*T-rustdoc*."

[notify-zulip."beta-nominated".compiler]
required_labels = ["T-compiler"]
zulip_stream = 266221
topic = "beta-nominated: #{number}"
message_on_add = "PR #{number}'s beta-nomination has been added."
message_on_remove = "PR #{number}'s beta-nomination has been removed."
message_on_close = "PR #{number} has been closed. Thanks for participating!"
message_on_reopen = "PR #{number} has been reopened. Pinging @*T-compiler*."

[notify-zulip."beta-nominated".miri]
required_labels = ["T-miri"]
zulip_stream = 266222
topic = "beta-nominated: #{number}"
message_on_add = "PR #{number}'s beta-nomination has been added."
message_on_remove = "PR #{number}'s beta-nomination has been removed."
message_on_close = "PR #{number} has been closed. Thanks for participating!"
message_on_reopen = "PR #{number} has been reopened. Pinging @*T-miri*."

According to the config above, for example, if an issue with the labels beta-nominated, T-rustdoc and T-miri opened, the streams 266220 and 266222 would be notified simultaneously.

It is backward-compatible, there is no need to change any current configuration. New notify-zulip items should have names like above.

Note: This is just a draft PR. I believe my approach is right but I am not confident on the implementation so I need your feedback. Another thing is that I forgot to update the docs. =)

[apiraino]

hey @ismailarilik thank you for this contribution. This was actually something we in T-compiler wanted (was on my TODO list...), see Zulip discussion.

One thing I can spot at a first look is that our discussion was favorable to publish these notification in a substream (e.g. T-compiler/backports instead of T-compiler). Let's remember about this detail when this change is merged and deployed, a specific stream needs to be created (maybe I can even do that, need to check my permissions) 🙂

cc @rust-lang/rustdoc so they can express their preference either

EDIT: created the Zulip channel for the T-compiler backport notifications
Link: https://rust-lang.zulipchat.com/#narrow/channel/474880-t-compiler.2Fbackports
Channel id: 474880

[aDotInTheVoid]

From a rustdoc side, my personal preference would be to keep those notifications in the main stream. It's low trafic enough I don't think it's worth (us) making a new stream for it, although I can imagine this being different for a larger/higher traffic team like compiler.

Also relevant context: rust-lang/rust#116957

[ismailarilik]

I was thinking that the docs were in that repo but it seems that they are in the Forge. I will update it after this PR is merged. BTW, here is the related page in Forge: https://forge.rust-lang.org/triagebot/zulip-notifications.html

[ismailarilik]

@rustbot review

[rustbot]

Error: The feature shortcut is not enabled in this repository.
To enable it add its section in the triagebot.toml in the root of the repository.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[ismailarilik]

How can I request a review? =)
I couldn't see a CONTRIBUTING.md so couldn't figure this out. Maybe I should just wait someone to pick this up and review. Sorry if that is the case.

[ismailarilik]

Sorry for the noise but do you know how I can test this appropriately? I setup triagebot in my local by following README and tried this implementation; it seems that it is working. But of course, I didn't try to send any Zulip notification to a team. Even if I tried this, I couldn't totally ensure that it would work on production.

[apiraino]

@ismailarilik I'll have a look at your patch later. Maintainers of this repo are listed here (not an obvious place), you can ping any of them. Please allow some time for a review.

how I can test this appropriately

To test the Zulip part one needs a Zulip instance. The quickest way is probably to register a new free instance for testing purposes.

r? @ehuss

[apiraino]

I gave a first look and this looks good to me :)

[GuillaumeGomez]

From a rustdoc side, my personal preference would be to keep those notifications in the main stream. It's low trafic enough I don't think it's worth (us) making a new stream for it, although I can imagine this being different for a larger/higher traffic team like compiler.

Also relevant context: rust-lang/rust#116957

Another rustdoc point of view: I agree. :)

[apiraino]

@ismailarilik I am working on making it possible for the triagebot using another Zulip instance for local testing. See my work-in-progress patch, feel free to test it, if you like (I didn't yet).

Edit: my patch has been merged

[apiraino]

I've tested this on a test Zulip instance (by manually applying this patch) and it seems to work.

[Urgau]

LGTM.

[Urgau]

Thanks for the continued effort.

I intend to merge it tomorrow (or Saturday).

cc @Kobzol (in case you want to take a look)