rust-lang / triagebot

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

Add a new `users_on_vacation` config which prevents PR assignment #1712

Closed jyn514 closed 11 months ago

jyn514 commented 11 months ago

People keep assigning me to PRs (usually new contributors who haven't seen my Zulip message). I very much do not want this. Prevent it in triagebot.

jyn514 commented 11 months ago

also i have github notifications off, so if you have feedback get Pietro to dm me on discord or something, i probably won't see it here

apiraino commented 11 months ago

Worth mentioning that there is an ongoing effort (lead by me) to solve this and other issues related to PR assignment. Here's my work in progress and a document detailing the reasoning and the global picture behind the idea.

jyn514 commented 11 months ago

thanks. the two PRs are complimentary i think - your PR tries to do automatic load balancing of auto assigning PRs based on capacity, mine is about preventing manual assignment so that it is impossible for me to get assigned to PRs. right now the only way to prevent manual assignment is to leave all rust teams altogether so i am not part of the rust-lang org.

apiraino commented 11 months ago

mine is about preventing manual assignment so that it is impossible for me to get assigned to PRs

that is true. By design I decided (after a first round of feedbacks) to allow the manual assignment (r? user) to override the time off settings of a contributor. But now I am thinking whether to rethink this and maybe have an additional ...

... kind of flag.

I think in future it's better to have one single place to manage these preferences and these .toml files don't allow the granularity I am looking for.

EDIT: What I am trying to say is that personally I see this as a quick fix to solve your problem to be perhaps reverted when the new machinery rolls out.

jyn514 commented 11 months ago

EDIT: What I am trying to say is that personally I see this as a quick fix to solve your problem to be perhaps reverted when the new machinery rolls out.

i'm fine with that yes, i just don't want to wait until your giant change merges.

workingjubilee commented 11 months ago

that is true. By design I decided (after a first round of feedbacks) to allow the manual assignment (r? user) to override the time off settings of a contributor. But now I am thinking whether to rethink this and maybe have an additional ...

It is ultimately a matter of consent. In theory, we can say the r+ bit comes with the understanding that you can be assigned to do a PR, even on holidays, even if your dog died that day, even in the event of a natural disaster, et cetera, and thus not give it out without consent to that.

In practice, we may find it contributes more to confusion if a PR can be assigned to someone who, say, took a moment to set a flag on GitHub or via a .toml or whatever, because they are extremely conscientious and also know they are about to spend at least the next month or so dealing with a suddenly-ignited war or revolution in their home country. It may discourage people from being so conscientious if they know the impact would be minimal anyways and not prevent confusion.

apiraino commented 11 months ago

In practice, we may find [that overriding a user's setting] contributes more to confusion if a PR can be assigned to someone who [...] took a moment to set a flag on GitHub or via a .toml or whatever [...]

Yeah, if someone has set themself as unavailable, it would be a saner assumption to ignore possible scenarios to override this explicit will (as explained on the HackMD doc, my reasoning was that a "r? user" implies a prev. conversation between the PR author and user somewhere else).

Thanks @workingjubilee for your thoughts!

rustbot commented 11 months ago

Error: Parsing assign command in comment failed: ...' user' | error: specify user to assign to at >| '" implies '...

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.