DefinitelyTyped
Allow owners to unsubscribe from some PRs
- Fixes #460
I'm sure this is incomplete, I'm just showing the suggested path forward. If accepted I can complete it.
Most likely you're going to need to run something like this to fix the tests, which snapshot replies to PRs.
$ AUTH_TOKEN=$(gh auth token) npm run update-all-fixtures
(But, after the comment threads are addressed, e.g. the "It's been a few days" does not to be changed)
the "It's been a few days" does not to be changed
No because that exclusively affects people who have unsubscribed, as shown in my other "review" comment.
However I think most mentions (ahem re-subscriptions) in StalenessComments are still there: https://github.com/fregante/dt-mergebot/blob/patch-1/src/comments.ts#L152
I think it's fine for now, later we can see if it's worth making further changes.
Most likely you're going to need to run something like this to fix the tests, which snapshot replies to PRs.
Snapshots updated 🫡 Tests pass locally 🟢 PR ready for merge 🚢
Thank you for still being here to discuss this with me 🙏
Just to generally chime in here, I'm not wild on these changes
I think our biggest problem as DT maintainers is folks with real context not responding to these pings and removing the direct mentions in a bunch of places makes it more likely for them to be ignoring the API response. Like Jake, I have not heard people complain about this outside of the issue you reported and I'm not super convinced that we should be changing this.
@orta please read the conversation we had so far. People will still be notified. This only affects people who have clicked Unsubscribe after the first notification.
The question you have to answer when merging/discarding this PR is: do I want to send a notification to people who specifically clicked Unsubscribe on the current PR?
That's all.
It has been more than a year and this PR still has no reviews.
I'll bump it to the DT maintainer queue. Thank you for your patience, @jakebailey @sandersn @andrewbranch.