search

rust-lang / triagebot

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

Ping new reviewer when `r? group` is used #1823

Open xFrednet opened 2 weeks ago

xFrednet commented 2 weeks ago

In the clippy repo we sometimes use r? clippy to reassign PRs. I've been triaging stale PRs and noticed that some of the new reviewers didn't notice their assignment. I assume this happens since r? clippy doesn't ping them. Could we add an automatic comment that pings the new reviewer?

xFrednet commented 2 weeks ago

I'd be happy to work on this. I see two options to implement this:

  1. Add a comment like

    Hey @ \<new-reviewer> you were chosen from \<group>. Could you take a look at this PR? Otherwise please reassigning it with r? <group>

  2. Change r? group to trigger a new comment with r? @<new-reviewer> that then assigns the new person

Is there any preference which option should be chosen?

apiraino commented 1 week ago

My understanding was that r? <team> indirectly triggers a notification to the new assignee of a pull request. An email and/or a new entry in the personal notification GH panel (depending on user configuration).

What makes you think this is not the case? Asking because it's something so fundamental that I gave it for granted :smile:

xFrednet commented 1 week ago

That was also my assumption. Maybe it's a bug? The notification reason is sometimes a bit inconsistent in my inbox. (Sometimes saying "assigned" and sometimes "mentioned" )

I'm not at home this weekend, but i can test this next week in more detail.

xFrednet commented 1 week ago

I've tested this and it seems like assignments do cause a GH notification :thinking: I guess we can close this then?

One thing I considered, was to have an automated ping or following reassign, if a PR hasn't gotten any attention, but that might be very hard to automate. :thinking:

apiraino commented 1 week ago

thanks for getting back @xFrednet . yes, I think we can close this.

automated ping or following reassign, if a PR hasn't gotten any attention

If it's of any consolation, I've been doing manual bookeeping of open PRs waiting on a review for ~2 years at T-compiler meetings :smile: and I still did not find a good automation that covers most of the corner cases.

I do have some code that queries the GH APIs and pull them more or less sorted like I want and that's already helpful. If you're seriously down to tackle this problem for T-clippy, let's talk about that on Zulip! :slightly_smiling_face:

xFrednet commented 1 week ago

Yeah, those things sadly tend to happen. The ideal solution would be to just increase the bandwidth of the teams. The word "just" is doing all the heavy lifting in this case ^^.

I really want this for new contributors. Waiting on your first PR is super discouraging.

If you're seriously down to tackle this problem for T-clippy, let's talk about that on Zulip! 🙂

I actually would be! I have one bigger project, that I want to make PR ready first. That might take two weeks or so, then I would reach out. What is your Zulip handle?