Skip to content

Give user feedback on backport Zulip command #2363

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

Merged
apiraino merged 1 commit into from
Apr 3, 2026
+9 −1
Merged

Give user feedback on backport Zulip command #2363

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
10 changes: 9 additions & 1 deletion src/zulip.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,15 @@ async fn handle_command<'a>(
}
StreamCommand::DocsUpdate => trigger_docs_update(message_data, &ctx.zulip),
StreamCommand::Backport(args) => {
accept_decline_backport(&ctx, message_data, &args).await
let _ = match accept_decline_backport(&ctx, message_data, &args).await {
// give user feedback
Ok(_) => ctx.zulip.add_reaction(message_data.id, "check").await,
Err(err) => {
log::error!("Could not handle backport #{}: {:?}", args.pr_num, err);
Copy link
Copy Markdown
Member

@Kobzol Kobzol Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that using the emoji is useful, but before triagebot would actually answer on the Zulip topic with the full error message; now the error is just logged. Shouldn't we do both?

Copy link
Copy Markdown
Contributor Author

@apiraino apiraino Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of an error a 😱 emoji is posted (lol).

In #2348 you suggested to just log. Do you want to also post the error message?

(I'm fine with either, thought that just an emoji reaction is more succint and less invasive - though maybe confusing)

Copy link
Copy Markdown
Member

@Kobzol Kobzol Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, for some reason I thought that #2348 did not return the full error message before you added the emoji. But it's weird - the code looks like it should have returned the error, but you saw on Zulip that no error was returned, which caused you to add the emoji.

Well, since we already did it there, let's follow suit here.

Copy link
Copy Markdown
Contributor Author

@apiraino apiraino Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I was too late.

Thought it was easier to acknowledge your comment so I have changed the code for both the "assign priority" and "approve backport" commands, now an error string is returned on Zulip (in addition to the emoji).

ctx.zulip.add_reaction(message_data.id, "scream").await
}
};
Ok(None)
}
StreamCommand::UserInfo {
username,
Expand Down