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(ApplicationCommand): default option.required to false #5838

Merged
merged 5 commits into from Jun 13, 2021

Conversation

ImRodry
Copy link
Contributor

@ImRodry ImRodry commented Jun 12, 2021

Please describe the changes this PR makes and why it should be merged:
The data sent by Discord for the required field in ApplicationCommandOption is undefined if the option is not required. This PR fixes this so that it defaults to false instead to make it easier to compare with local commands for command updaters.

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

@ImRodry
Copy link
Contributor Author

ImRodry commented Jun 12, 2021

This didn’t cross my mind when making the PR but SUB_COMMAND and SUB_COMMAND_GROUP types cannot have the required value set so I’ll need to do some adjusting to catch those cases and set it to undefined then

@ImRodry ImRodry closed this Jun 13, 2021
@ImRodry ImRodry deleted the patch-1 branch June 13, 2021 10:05
@ImRodry ImRodry restored the patch-1 branch June 13, 2021 10:06
@ImRodry ImRodry reopened this Jun 13, 2021
@ImRodry
Copy link
Contributor Author

ImRodry commented Jun 13, 2021

Oopsie

ImRodry added a commit to BanTheNons/hypixel-translators-bot that referenced this pull request Jun 13, 2021
Just need to wait for discordjs/discord.js#5838 to be merged so that I can get rid of the bad patch in ready.ts line 141
@iCrawl iCrawl merged commit 77c1f15 into discordjs:master Jun 13, 2021
@ImRodry ImRodry deleted the patch-1 branch June 13, 2021 18:31
@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.

None yet

5 participants