Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop pinging owners over and over for the same PR (allow unsubscribing) #460

Open
fregante opened this issue Apr 12, 2023 · 5 comments · May be fixed by #461
Open

Stop pinging owners over and over for the same PR (allow unsubscribing) #460

fregante opened this issue Apr 12, 2023 · 5 comments · May be fixed by #461

Comments

@fregante
Copy link

fregante commented Apr 12, 2023

Pings are necessary to make people aware of a new PR. If they don't take action, every following comment is already delivered to their notification by default. Continuous pinging doesn't do anything extra.

I want owners to have two options:

  • do nothing or review/comment (keep receiving updates)
  • click unsubscribe (stop receiving updates)

The second option is not available because the bot keeps bugging everyone in nearly every comment.

@fregante fregante linked a pull request Apr 12, 2023 that will close this issue
@fregante
Copy link
Author

fregante commented Jan 16, 2024

@jakebailey @sandersn @andrewbranch

This bot behavior leads to people unsubscribing/blocking DefinitelyTyped, causing them to never contribute to the project again. This is the opposite of what this feature was meant to achieve.

Please review this bot and my open PR, it's a very straightforward change.

@jakebailey
Copy link
Member

I'm not sure I agree with your premise. Getting pinged is exactly what a DT owner signed up for; why be an owner if you're not willing to review PRs?

If you're adamant to not re-ping, then either we should always ping initially, or potentially even just check who all has been previously pinged in the PR and only ping new owners (allowing older PRs to continue to ping the most recent people). Your current PR seems to remove the initial ping, which seems incorrect.

(It is of course funny to be pinged here about not wanting to be pinged 😄)

@fregante
Copy link
Author

🙌 We agree on the base premise, I opened with this:

Pings are necessary to make people aware of a new PR

The issue is not about being pinged, but allowing unsubscribing to each PR.

A single mention is enough to:

  • notify owners of new PRs
  • notify owners of changes to the PR, unless they explicitly unsubscribed from the PR

Let's say a PR is opened regarding a part of a package that I've never even seen, or that I have no time to review. I want to unsubscribe from that specific PR and not be notified again.

Because let's say Andrew saw this issue and and clicked "Unsubscribe" because he's on vacation, but I'm a bot and I'll mention y'all again:

@jakebailey @sandersn @andrewbranch

…and maybe I'll mention every one again every comment. I bet I'll be blocked after the 3rd mention here. 🥹

(It is of course funny to be pinged here about not wanting to be pinged 😄)

I hope that gets the message across 😃

@fregante
Copy link
Author

check who all has been previously pinged in the PR

  • Do PRs live long enough (on average) that new owners are added while they're open?
  • Are owners added often enough?

The rarity of this subset of PR/owners makes me think it's not worth the extra complexity of this.

@jakebailey
Copy link
Member

Because let's say Andrew saw this issue and and clicked "Unsubscribe" because he's on vacation, but I'm a bot and I'll mention y'all again:

Unsubscribing from specific threads is certainly not the method I'd use to silence notifications on vacation. There are much better tools for that. But this is moot.

The rarity of this subset of PR/owners makes me think it's not worth the extra complexity of this.

Yes, that was an over the top suggestion. Simply making it ping only in the first comment would be sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@fregante @jakebailey and others