-
Notifications
You must be signed in to change notification settings - Fork 96
Accept/decline T-compiler backports from Zulip #2155
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
base: master
Are you sure you want to change the base?
Conversation
a42cf9d
to
b265ac9
Compare
}, | ||
content: "", | ||
}; | ||
let zulip_link = zulip_send_req.url(zulip_client); |
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.
Note: for how the Message API works today, we can't pin exactly a single message so the link in the GitHub comment will be to the whole topic, which is a bit inconvenient but hopefully will be solved by Zulip. I'd rather wait on them for a correct fix rather us add hacks to make it work.
b265ac9
to
80d4110
Compare
I guess this patch went under the radar because I didn't request a review r? triagebot |
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.
Sounds pretty useful!
#[derive(clap::Parser, Debug, PartialEq, Clone)] | ||
pub struct BackportArgs { | ||
/// Release channel this backport is pointing to. Allowed: "beta" or "stable". | ||
pub channel: String, |
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.
Could you please use an enum
here for the channel and the verb, and either generate the parser with #[derive(clap::ValueEnum)]
, or write it manually with FromStr
? That way we won't have to check for invalid data in the backport handler, and we'll get a nice error message and a hint (in case of misspelling) for free.
05aa91f
to
6d99fff
Compare
5861437
to
f32c8db
Compare
f32c8db
to
f373c3c
Compare
This patch adds a new Zulip command:
@triagebot backport <channel> <action> <PR>
Example:
@triagebot backport stable accept 123456
@triagebot backport beta decline 654321
This can be used to post on GitHub a comment like this one when T-compiler accepts/decline to backport a patch that fixes a regression.
Limitations:
rust-lang/rust
, I am not aware of other git repos having this workflow. If we want, maybe in the future we can extend to other repos using our custom Zulip linkfiersTested on a custom Zulip instance.
TODO:
backport accept
or evenaccept
) to be used from#t-compiler/backports
(topics contain all necessary info to build the command parameters)