search

rust-lang / triagebot

Automation/tooling for Rust spaces
https://triage.rust-lang.org
Apache License 2.0
170 stars 75 forks source link

Add ability to trigger docs-update via Zulip #1702

Closed ehuss closed 1 year ago

ehuss commented 1 year ago

This adds the ability to trigger a docs-update via Zulip with the message @triagebot docs-update.

This also includes two other commits. The first reworks the API of the zulip module to try to simplify it. From what I can tell, 100% of the time the API was returning a message to display to the user, but each time it was manually serializing a message to send. This changes it so that the API uses anyhow::Result<String> instead, where the returned String is the string to post as the reply, and if there is an error it should send a response with the text of the error. I can drop this commit if I'm missing some reason for why it was done this way. There shouldn't be any real behavior changes here.

I could imagine there was maybe some desire to have different kinds of responses? I imagine if this ever needs to respond with response_not_required we could change it to Result<Option<String>> to implement that. Or maybe there were concerns about how errors were being displayed, and this wanted to be more explicit about sending error responses? In practice it seemed like it always showed a debug display of the error anyways.

There is a third commit here which changes the merge-upstream API to do a force update if there is an error. This matches the behavior of the gh CLI. I can understand the 409 Conflict error which is a typical merge conflict. However, the error we are currently experiencing 422 Unprocessable Entity, and I don't know why. The gh CLI also handles this error with a force update, so I figure it might be worth to give it a try and see if it works.

Unfortunately I'm not able to test the Zulip changes since it looks like I'm not able to create a bot in order to set up a webhook.

Mark-Simulacrum commented 1 year ago

I think this all makes sense -- back when we added the Zulip logic, I think I only later realized that the API doesn't really give you much in the way of response options. For example, if you want to add emojis you can't do that directly, since the webhook response doesn't have those fields. (Or at least this is my loose recollection of what I recall from back then).