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

fix(ReactionUserManager): remove before query option #5281

Merged
merged 1 commit into from
Mar 28, 2021

Conversation

almostSouji
Copy link
Member

@almostSouji almostSouji commented Feb 1, 2021

Please describe the changes this PR makes and why it should be merged:

The before option is apparently not actually supported as per discord/discord-api-docs#1324 (comment) (despite documented)

Status and versioning classification:

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating
  • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

@papaia
Copy link
Contributor

papaia commented Feb 1, 2021

This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

But you labeled it semver: patch.

@almostSouji
Copy link
Member Author

Technically breaking, because it removes something from the interface
But it is a bug fix, seeing that it removes an unsupported option

Unsure which applies.

@papaia
Copy link
Contributor

papaia commented Feb 1, 2021

The behavior won't change as that param is not respected by Discord.

@almostSouji

This comment has been minimized.

@tguichaoua
Copy link
Contributor

Technically breaking, because it removes something from the interface
But it is a bug fix, seeing that it removes an unsupported option

Unsure which applies.

In TypeScript, it introduce a breaking change.
playground

@papaia
Copy link
Contributor

papaia commented Feb 7, 2021

Pretty sure we don't count typescript breaks as breaking.

@iCrawl iCrawl merged commit 43bd568 into discordjs:master Mar 28, 2021
@almostSouji almostSouji deleted the rum-remove-before branch April 6, 2021 20:19
@iCrawl iCrawl added this to the Version 13 milestone Jun 27, 2021
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 this pull request may close these issues.

6 participants