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

feat(*): use enums for consistency and speed #5843

Merged
merged 6 commits into from Jun 15, 2021

Conversation

iShibi
Copy link
Contributor

@iShibi iShibi commented Jun 13, 2021

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

Okay, there are a lot of related changes here:

  1. Use enums instead of arrays. This brings consistency and speed (indexOf is slow).
  2. Some enums (that were arrays earlier) had same name for the enums themselves and the type of elements they hold.
  3. WebhookTypes enum (ex-array) was missing from typings for Constants
  4. Some methods that used arrays as enums earlier wouldn't let user to pass number type.
  5. DefaultMessageNotifcations is now DefaultMessageNotificationLevels to keep the naming consistent with other enums in djs (discord uses 'level' in the name for enums in documentaion).
  6. DefaultMessageNotifcations types were not same as what the api has documented ( eg. ALL instead of ALL_MESSAGES)
  7. Although, the raw activity data's name property's value is Custom_Status, the type for it is documented as Custom. So, the activity type CUSTOM_STATUS is now CUSTOM.

Status and versioning classification:

  • Code changes have been tested against the Discord API
  • I know how to update typings and have done so
  • This PR includes breaking changes

src/managers/GuildManager.js Outdated Show resolved Hide resolved
Copy link
Member

@kyranet kyranet left a comment

Choose a reason for hiding this comment

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

Sorry, wrong button... ^

src/util/Constants.js Show resolved Hide resolved
src/util/Constants.js Show resolved Hide resolved
src/util/Constants.js Show resolved Hide resolved
src/structures/Guild.js Outdated Show resolved Hide resolved
src/structures/Guild.js Outdated Show resolved Hide resolved
src/structures/Guild.js Outdated Show resolved Hide resolved
@iCrawl
Copy link
Member

iCrawl commented Jun 15, 2021

This needs a rebase.

@iShibi
Copy link
Contributor Author

iShibi commented Jun 15, 2021

Rebased

@iCrawl iCrawl merged commit f7eeccb into discordjs:master Jun 15, 2021
@iShibi iShibi deleted the only-enums branch June 15, 2021 15:00
@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