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

docs: general cleanup and improvements #6299

Merged
merged 8 commits into from
Aug 5, 2021

Conversation

ImRodry
Copy link
Contributor

@ImRodry ImRodry commented Aug 4, 2021

Please describe the changes this PR makes and why it should be merged:
I went through every page on the docs and added/fixed some things. Although the title says docs this does include very small changes to the typings and code which I will be detailing below and you're welcome to give your opinion on them if you believe they should be reverted.

⚠️ This PR contains the following breaking changes:

  • Removed the RELAY_ENABLED guild feature as it was not found on the docs
  • Marked Guild#preferredLocale as nullable since a Guild may not have community enabled
  • Removed the MANAGED_EMOJI, GROUP_DM_CREATE and RPC_HAS_CONNECTED Application flags as they could not be found on the docs
  • Changed the default format for the Emoji CDN endpoint to 'webp' to match the other endpoints and the docs
  • Renamed the PINS_ADD message type to CHANNEL_PINNED_MESSAGE to match the Discord API docs

Along with those, here are the other changes:

  • Added the Application Webhook type
  • Added back the Collection @external link
  • Fixed some wrong string types where they should be Snowflake
  • Removed nested properties (e.g. options.x) and added them to separate typedefs
  • Added monospace markdown to references to properties within typedefs
  • Added mentioned class links in multiple descriptions
  • Fixed a weird code example in the guild ban and kick methods
  • Changed all "Promise<T>|Promise<T2>" types to "Promise<T|T2>"
  • Removed redundant @returns descriptions
  • Fixed typos
  • Added links to Discord documentation in most enums whenever I was able to find them
  • Moved a sentence in the Snowflake typedef out of the code block to prevent formatting issues (not sure if this will cause other issues, please let me know if it does)
  • Created the DynamicImageFormat type and assigned it to the CDN endpoints that support dynamic images (including DynamicImageFormat which supported fewer types before)
  • Renamed the ImageSize type to AllowedImageSize for consistency and assigned it to all CDN endpoint sizes (change previously suggested in feat(GuildMember): add guild avatars #5696 but might as well do it now).

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)

src/util/BitField.js Outdated Show resolved Hide resolved
src/util/BitField.js Outdated Show resolved Hide resolved
src/util/BitField.js Outdated Show resolved Hide resolved
src/sharding/ShardingManager.js Outdated Show resolved Hide resolved
src/structures/InteractionCollector.js Outdated Show resolved Hide resolved
src/structures/MessageEmbed.js Outdated Show resolved Hide resolved
src/structures/PermissionOverwrites.js Outdated Show resolved Hide resolved
src/util/Constants.js Outdated Show resolved Hide resolved
Co-authored-by: DaStormer <40336269+DaStormer@users.noreply.github.com>
src/managers/GuildBanManager.js Outdated Show resolved Hide resolved
src/managers/GuildMemberManager.js Outdated Show resolved Hide resolved
src/sharding/ShardClientUtil.js Outdated Show resolved Hide resolved
src/sharding/ShardingManager.js Show resolved Hide resolved
src/util/LimitedCollection.js Outdated Show resolved Hide resolved
Copy link
Member

@iCrawl iCrawl left a comment

Choose a reason for hiding this comment

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

Apart from the changes requested 👍

ImRodry and others added 2 commits August 5, 2021 14:47
Co-authored-by: Sugden <28943913+NotSugden@users.noreply.github.com>
@ImRodry
Copy link
Contributor Author

ImRodry commented Aug 5, 2021

Seems like the CI got stuck, can someone rerun it? It took long but we got there

src/managers/MessageManager.js Outdated Show resolved Hide resolved
src/sharding/ShardClientUtil.js Outdated Show resolved Hide resolved
src/sharding/ShardingManager.js Outdated Show resolved Hide resolved
src/sharding/ShardingManager.js Outdated Show resolved Hide resolved
src/sharding/ShardingManager.js Outdated Show resolved Hide resolved
src/util/ApplicationFlags.js Show resolved Hide resolved
src/util/LimitedCollection.js Outdated Show resolved Hide resolved
Co-authored-by: SpaceEEC <spaceeec@yahoo.com>
@ImRodry ImRodry requested a review from SpaceEEC August 5, 2021 15:23
@ImRodry
Copy link
Contributor Author

ImRodry commented Aug 5, 2021

The CI is just broken today apparently

@SpaceEEC
Copy link
Member

SpaceEEC commented Aug 5, 2021

There seems to be a partial outage, although the workflow is now at least queued.

src/managers/GuildBanManager.js Outdated Show resolved Hide resolved
src/managers/GuildMemberManager.js Outdated Show resolved Hide resolved
src/managers/GuildMemberManager.js Outdated Show resolved Hide resolved
src/structures/ClientUser.js Show resolved Hide resolved
src/structures/GuildBan.js Outdated Show resolved Hide resolved
src/structures/VoiceState.js 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.

Besides the requested changes, LGTM

@iCrawl iCrawl requested a review from vladfrangu August 5, 2021 19:14
@iCrawl iCrawl merged commit b4afcf8 into discordjs:master Aug 5, 2021
@Nytelife26
Copy link

Some textual errors in this PR, namely writing id instead of ID.

@ImRodry ImRodry deleted the docs-general-cleanup branch August 5, 2021 19:44
@ImRodry
Copy link
Contributor Author

ImRodry commented Aug 5, 2021

Some textual errors in this PR, namely writing id instead of ID.

Both are fine but if you wanted that changed you should’ve probably said that before merge

@DaStormer
Copy link
Contributor

id is used throughout now anyway, refer to #6036

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.