Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Aug 6, 2020

Please describe the changes this PR makes and why it should be merged:
ReactionManager#cache returns a collection of MessageReaction mapped by either a Snowflake (for custom emojis) or a string(for emojis); but current type of ReactionManager#cache is Collection<Snowflake, MessageReaction>.
Even if Snowflake is just a named typedef for string, it still seems a bit misleading and semantically incorrect.

Status

  • 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

Semantic versioning classification:

  • This PR changes the library's interface (methods or parameters added)
    • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
  • This PR only includes non-code changes, like changes to documentation, README, etc.

@vaporoxx
Copy link
Contributor

vaporoxx commented Aug 6, 2020

Snowflake is typed as just string, so Collection<string, MessageReaction> would be sufficient in my opinion. Also, you would need to change the typings here (i.e. BaseManager<string, ...) before the "Updated typings" box can be checked.

@ghost
Copy link
Author

ghost commented Aug 6, 2020

Also, you would need to change the typings here (i.e. BaseManager<string, ...) before the "Updated typings" box can be checked.

uh, I didn't knew I had to do it. Done, thank you.

@iCrawl iCrawl merged commit 5b716c5 into discordjs:master Aug 11, 2020
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.

4 participants