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

refactor: remove obsolete builder methods #7590

Merged
merged 3 commits into from Mar 6, 2022

Conversation

almeidx
Copy link
Member

@almeidx almeidx commented Mar 2, 2022

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

  • Removed ApplicationCommandOptionChannelTypesMixin#addChannelType()
  • Removed ApplicationCommandOptionWithChoicesAndAutocompleteMixin#addChoice()
  • ApplicationCommandOptionChannelTypesMixin#addChannelTypes() only accepts rest parameters instead of an array

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 changes the library's interface (methods or parameters added)
  • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

@almeidx almeidx marked this pull request as ready for review March 2, 2022 20:29
@vvito7
Copy link
Contributor

vvito7 commented Mar 2, 2022

Shouldn't it also be changed in Slash Command Builders.md?

.addChoices([
['Add points', 'add'],
['Remove points', 'remove'],
['Reset points', 'reset'],
])

Copy link
Member

@vladfrangu vladfrangu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never liked this removal of singular vs multiple additions but 🤷 LGTM

@iCrawl iCrawl merged commit 10607db into discordjs:main Mar 6, 2022
@almeidx almeidx deleted the builders-cleanup branch March 6, 2022 15:30
@aedwardg
Copy link

aedwardg commented Jun 4, 2022

Considering this message and the fact that builders 0.13.0 is required for upgrading to 13.7.0, it would have been nice if this breaking change were communicated better.

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

Successfully merging this pull request may close these issues.

None yet

6 participants