Give user feedback on backport Zulip command#2363
Conversation
| // 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); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
|
I wonder if we should maybe do this in a more general way, i.e. for all stream commands, if they are successful, mark them with an emoji, and if they fail, use a different emoji, to mark the "result" of the command. |
Good point but I'd need to review all Zulip commands, I think some already return a textual feedback to the user. I am unsure about refactoring all of them because people are used to a behaviour and changing that without asking feels mildly hostile (even if with good intentions 🙂 ) |
2 participants
Similar to #2348, I've added an "emoji user feedback" also when approving/declining backports in T-compiler triage meetings, that's really useful!
r? @Kobzol
thanks