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

Base Emoji class for ReactionEmoji and renamed GuildEmoji classes #2230

Merged
merged 2 commits into from
Jan 18, 2018

Conversation

SpaceEEC
Copy link
Member

@SpaceEEC SpaceEEC commented Jan 4, 2018

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

Create new Emoji class as base for GuildEmoji and ReactionEmoji to avoid code duplication

Summary:

  • Renamed Emoji to GuildEmoji, refelects in Structures
  • Created new Emoji class to avoid code duplication between GuildEmoji and ReactionEmoji
  • Both GuildEmoji and ReactionEmoji extend said Emoji class
  • Renamed EmojiStore to GuildEmojiStore to stay consistent with the class' name GuildEmoji

TODO:

  • Verify all docstrings are correct with new/renamed classes
  • Verify all instanceofs and similar are correct
  • Verify reacting / removing of such works properly
  • Verify all received emojis are in their proper places and classes (GuildEmoji, ReactionEmoji)
  • Verify resolving of Reaction and GuildEmojis works properly

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.

@SpaceEEC SpaceEEC changed the title add ReactionEmoji#animated and prepend a: when necessary WIP: GuildEmoji / ReactionEmoji extends Emoji Jan 4, 2018
@SpaceEEC SpaceEEC changed the title WIP: GuildEmoji / ReactionEmoji extends Emoji Base Emoji class for ReactionEmoji and renamed GuildEmoji classes Jan 5, 2018
@@ -1,15 +1,15 @@
const DataStore = require('./DataStore');
const Emoji = require('../structures/Emoji');
const GuildEmoji = require('../structures/GuildEmoji');
const ReactionEmoji = require('../structures/ReactionEmoji');

/**
* Stores emojis.
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit obscure.. Isn't there a store for the main client? If there is, this should state that it stores guild emojis (unless client uses a collection in which case :P)

@Lewdcario Lewdcario merged commit aa3407f into discordjs:master Jan 18, 2018
vladfrangu pushed a commit to vladfrangu/discOwOd.js that referenced this pull request Jan 18, 2018
…scordjs#2230)

* feat: create base Emoji class for ReactionEmoji and new GuildEmoji

* rename EmojiStore to GuildEmojiStore to account for the new class' name
vladfrangu pushed a commit to vladfrangu/discOwOd.js that referenced this pull request Jan 18, 2018
@SpaceEEC SpaceEEC deleted the animated_emoji branch January 18, 2018 19:38
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