search

DefinitelyTyped / dt-mergebot

The bot which handles auto-merging your PRs
https://devblogs.microsoft.com/typescript/changes-to-how-we-manage-definitelytyped/
MIT License
112 stars 44 forks source link

Allow owners to unsubscribe from some PRs #461

Open fregante opened 1 year ago

fregante commented 1 year ago

I'm sure this is incomplete, I'm just showing the suggested path forward. If accepted I can complete it.

jakebailey commented 9 months ago

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)

fregante commented 9 months ago

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 🙏

orta commented 9 months ago

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.

fregante commented 9 months ago

@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.

fregante commented 6 months ago

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.