Stream: t-release/triage

Topic: automatic rustbot info


view this post on Zulip bstrie (Jul 21 2021 at 20:28):

In the libs team meeting today it was mentioned that it would be nice to inform users that they can tag their PRs as waiting-on-review without having to ping a contributor (or more likely, they don't realize that it's even a thing and just end up waiting for a triage team member to do it). The proposal was that the very first time that a PR is tagged as waiting-on-author, rustbot automatically comments something like:

This PR is now waiting for changes from the author. When this PR is ready to be reviewed again, please leave a comment with the following to re-add this PR to the review queue:

@rustbot label -S-waiting-on-author +S-waiting-on-review

what do people think?

view this post on Zulip The 8472 (Jul 21 2021 at 20:58):

There's a @rustbot ready shorthand for that. I learned about that by seeing someone else use that... is there a cheat sheet of available commands?

view this post on Zulip bstrie (Jul 21 2021 at 21:04):

Sure, although the point here isn't which command to present to the user, it's that we should have some automatic process that ensures the user knows that they have the power to mark the PR as ready, so that they don't have to wait for the triage team to do it. This could take some load off the triage team and also reduce turnaround times for PRs. Also, some members of the libs team also mentioned that they also forget the rustbot syntax, so having such a reminder might be beneficial for more people than just newcomers

view this post on Zulip The 8472 (Jul 21 2021 at 21:08):

I know, I meant learning one command would be a start and help in this particular context, but a link to a cheat sheet might be even more useful.

view this post on Zulip Joshua Nelson (Jul 22 2021 at 00:26):

bstrie said:

In the libs team meeting today it was mentioned that it would be nice to inform users that they can tag their PRs as waiting-on-review without having to ping a contributor (or more likely, they don't realize that it's even a thing and just end up waiting for a triage team member to do it). The proposal was that the very first time that a PR is tagged as waiting-on-author, rustbot automatically comments something like:

This PR is now waiting for changes from the author. When this PR is ready to be reviewed again, please leave a comment with the following to re-add this PR to the review queue:

@rustbot label -S-waiting-on-author +S-waiting-on-review

what do people think?

I think this is an excellent idea and we should have done it ages ago :D

view this post on Zulip bstrie (Jul 24 2021 at 15:33):

what would the next step be towards pursuing this? should I open an issue on some repo? I have no idea where the bots live

view this post on Zulip Joshua Nelson (Jul 24 2021 at 15:34):

@bstrie it would go in https://github.com/rust-lang/triagebot/ somewhere I think

view this post on Zulip Joshua Nelson (Jul 24 2021 at 15:34):

I _think_ highfive only deals with the inital PR creation, not updates

view this post on Zulip bstrie (Jul 24 2021 at 16:01):

so does triagebot listen to the stream of all github events coming out of the rust repos? presumably such a thing is possible in the github API, and presumably it should be easy to listen for any one particular tag, although I don't know how difficult it would be to only do a thing only the first time that a tag is applied

view this post on Zulip bstrie (Jul 24 2021 at 16:03):

and I'm loathe to have this comment go off every time that a tag is applied, since we often remove and re-add these tags as a way of silently bumping github's "last time since activity happened" count

view this post on Zulip Joshua Nelson (Jul 24 2021 at 16:13):

Triagebot doesn't have a database that I know of, no. You could see if it had already published that comment on the PR though.

view this post on Zulip bstrie (Jul 24 2021 at 16:18):

yes, I don't want to have triagebot track this on its own in a database. good point that we could use the comment itself as a marker

view this post on Zulip bstrie (Jul 24 2021 at 16:36):

filed https://github.com/rust-lang/triagebot/issues/1449

view this post on Zulip DPC (Jul 25 2021 at 14:58):

maybe we could add the message to highfive message on top (when it auto assigns a reviewer)?

view this post on Zulip bstrie (Jul 25 2021 at 15:15):

that does solve the problem of only-send-a-single-message-per-PR and prevents potential spam, although I think it's easy to overlook and also easy to forget what it's told you since the waiting-on-author label might come long after the PR is filed. still, it would be an improvement. maybe in addition to specifically calling out rustbot ready, we could also link to some kind of rustbot command cheatsheet, if one exists

view this post on Zulip Noah Lev (Jul 27 2021 at 02:41):

Joshua Nelson said:

Triagebot doesn't have a database that I know of, no. You could see if it had already published that comment on the PR though.

Triagebot actually does have a database; see src/db.rs in the triagebot repo. Migrations are at the bottom of that file, and handlers for different features are in src/db/.

Also, if you have questions about how to implement things in triagebot, you can ask in #t-release/triagebot :)

view this post on Zulip bstrie (Jul 28 2021 at 19:39):

well if there's already a database that answers the question of how this could be done, now people just need to decide if they want it :)

view this post on Zulip Joshua Nelson (Jul 28 2021 at 20:43):

Yes, I think we should do this.

view this post on Zulip Noah Lev (Jul 29 2021 at 22:11):

I also think it's a good idea to let people know how they can update the status of their PR.

view this post on Zulip Noah Lev (Jul 29 2021 at 22:12):

But if rustbot comments every time a PR is put into waiting-on-author for the first time, I feel that would be annoying.

view this post on Zulip Noah Lev (Jul 29 2021 at 22:12):

Maybe we could do it only if it's someone's first, second, or third PR?

view this post on Zulip Noah Lev (Jul 29 2021 at 22:12):

Of course, the first time the feature was activated, everyone would get the comment, but it might help people who hadn't seen it before, and it would go away soon for frequent contributors.

view this post on Zulip Léo Lanteri Thauvin (Jul 29 2021 at 23:32):

Highfive already does send a message when first-time contributors open a PR, FWIW

view this post on Zulip Léo Lanteri Thauvin (Jul 29 2021 at 23:38):

https://github.com/rust-lang/highfive/blob/c1b314239752352475aa802b5ae5fcd40f6b6fc9/highfive/newpr.py#L422-L225

view this post on Zulip Noah Lev (Jul 29 2021 at 23:43):

Yes, but I think the idea here is to post the message when the PR is put in waiting-in-author state, not when it's opened, so that people notice it.

view this post on Zulip Joshua Nelson (Jul 30 2021 at 01:53):

Yup, we could do the same check for a first contributor and post when it goes into waiting-on-author

view this post on Zulip DPC (Jul 30 2021 at 03:15):

the problem i see with this is people might not know the pr is waiting for them (e.g. conflicts) and might keep updating it to waiting on review for it to end in a cycle

view this post on Zulip Joshua Nelson (Jul 30 2021 at 03:26):

Then someone can tell them not to do that :shrug: it doesn't seem that likely to me

view this post on Zulip Noah Lev (Jul 30 2021 at 22:38):

DPC said:

the problem i see with this is people might not know the pr is waiting for them (e.g. conflicts) and might keep updating it to waiting on review for it to end in a cycle

We could have the message say that it's likely waiting for them because

Also, bors usually comments saying that the PR has merge conflicts that need to be addressed.

view this post on Zulip bstrie (Jul 31 2021 at 20:31):

when a PR has merge conflicts, having triagebot automatically mark it as waiting-on-author sounds like a good idea

view this post on Zulip Léo Lanteri Thauvin (Jul 31 2021 at 20:33):

Isn't that already the case?

view this post on Zulip Léo Lanteri Thauvin (Jul 31 2021 at 20:34):

Nevermind it's not

view this post on Zulip Joshua Nelson (Aug 01 2021 at 06:33):

I don't agree that's always the case. Adding a new feature gate hits conflicts very rapidly, but usually they don't prevent reviewing, and you're waiting for an MCP to be accepted or something.

view this post on Zulip Joshua Nelson (Aug 01 2021 at 06:34):

So I don't think we should flip the status automatically, the reviewer should have to explicitly say "this changed enough on master I need you to rebase before I can review"

view this post on Zulip DPC (Aug 01 2021 at 18:37):

resolving the conflicts would require a rereview anywhere since we often hve a "anyone but the author" last check on prs


Last updated: Jan 26 2022 at 13:04 UTC