-
Notifications
You must be signed in to change notification settings - Fork 39
Make use of ES7 async when possible #122
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
Conversation
|
fix for #117 |
|
Just looked through c5ca908 and noticed there's a lot of async usage without actually awaiting anything. With help.js as example, did you mean to wait until each of those embeds are finished and only then return |
|
It's intended for help, that needs a further rewrite right now as it is, so i'm waiting for other enhancements on that one for afterward. |
TobiTenno
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple small changes or talking points for us.
Do we want to finish this pass on the commands, do the core utilities like database/settings and messageManager and then come back and do a second pass on the commands?
src/commands/Owner/Avatar.js
Outdated
| .then(() => this.messageManager.reply(message, 'New avatar set!', true, true)) | ||
| .catch(this.logger.error); | ||
| await this.bot.client.user.setAvatar(url); | ||
| this.messageManager.reply(message, 'New avatar set!', true, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you add the success returns for when it succeeds when the message is sent?
src/commands/Owner/LeaveServer.js
Outdated
| const guild = await this.bot.client.guilds.get(serverid).leave(); | ||
| this.messageManager.reply(message, `Left ${guild.name}`, true, true); | ||
| } else { | ||
| message.reply('No such guild cached'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similar to Avatar, could you have an ❌ response after 'No such guild cached'?
| const commandsRemovedString = commandsRemoved.length > 0 ? commandsRemoved.sort().join(' ') : ' No Commands Removed'; | ||
|
|
||
| this.messageManager.sendMessage(message, `${this.md.codeMulti}Commands reloaded!${this.md.blockEnd}` + | ||
| await this.messageManager.sendMessage(message, `${this.md.codeMulti}Commands reloaded!${this.md.blockEnd}` + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably want to look at returning the values from the operations in the messageManager functions so that these and successes make more sense.
| const longest = roles.map(role => role.name) | ||
| .reduce((a, b) => (a.length > b.length ? a : b)); | ||
| const groupedRoles = createGroupedArray(roles, 20); | ||
| const metaGroups = createGroupedArray(groupedRoles, 4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is more a criticism of my own code, but we could probably just wrap the second createGroupedArray call around the result of the first, since the first isn't used independently.
TODO: * Worldstate commands * database calls * message manager overhaul
- still need work to resolve conflicts on notifier class not recognizing functions
Removed double "custom commands" tokens await notifier starting
src/embeds/ProfileEmbed.js
Outdated
| ? `${player.marked.stalker ? '<:stalker:330021480169603076>' : ''} ` + | ||
| { | ||
| name: 'Current Mastery', | ||
| value: player.mastery.xp, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 10 spaces but found 8. (indent)
src/embeds/ProfileEmbed.js
Outdated
| { | ||
| name: 'Current Mastery', | ||
| value: player.mastery.xp, | ||
| inline: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 10 spaces but found 8. (indent)
src/embeds/ProfileEmbed.js
Outdated
| name: 'Current Mastery', | ||
| value: player.mastery.xp, | ||
| inline: true, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 8 spaces but found 6. (indent)
src/embeds/ProfileEmbed.js
Outdated
| value: player.mastery.xp, | ||
| inline: true, | ||
| }, | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 8 spaces but found 6. (indent)
src/embeds/ProfileEmbed.js
Outdated
| inline: true, | ||
| }, | ||
| { | ||
| name: 'Next Rank', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 10 spaces but found 8. (indent)
src/embeds/ProfileEmbed.js
Outdated
| }, | ||
| { | ||
| name: 'Next Rank', | ||
| value: `${player.mastery.xpUntilNextRank} until **${player.mastery.rank.next}**`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 10 spaces but found 8. (indent)
src/embeds/ProfileEmbed.js
Outdated
| { | ||
| name: 'Next Rank', | ||
| value: `${player.mastery.xpUntilNextRank} until **${player.mastery.rank.next}**`, | ||
| inline: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 10 spaces but found 8. (indent)
src/embeds/ProfileEmbed.js
Outdated
| name: 'Next Rank', | ||
| value: `${player.mastery.xpUntilNextRank} until **${player.mastery.rank.next}**`, | ||
| inline: true, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 8 spaces but found 6. (indent)
src/embeds/ProfileEmbed.js
Outdated
| value: `${player.mastery.xpUntilNextRank} until **${player.mastery.rank.next}**`, | ||
| inline: true, | ||
| }, | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 8 spaces but found 6. (indent)
src/embeds/ProfileEmbed.js
Outdated
| inline: true, | ||
| }, | ||
| { | ||
| name: 'Clan', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 10 spaces but found 8. (indent)
src/embeds/ProfileEmbed.js
Outdated
| }, | ||
| { | ||
| name: 'Clan', | ||
| value: player.clan.type ? `${player.clan.name}\nRank **${player.clan.rank}** ${player.clan.type}` : player.clan.name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 10 spaces but found 8. (indent)
src/embeds/ProfileEmbed.js
Outdated
| { | ||
| name: 'Clan', | ||
| value: player.clan.type ? `${player.clan.name}\nRank **${player.clan.rank}** ${player.clan.type}` : player.clan.name, | ||
| inline: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 10 spaces but found 8. (indent)
src/embeds/ProfileEmbed.js
Outdated
| name: 'Clan', | ||
| value: player.clan.type ? `${player.clan.name}\nRank **${player.clan.rank}** ${player.clan.type}` : player.clan.name, | ||
| inline: true, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 8 spaces but found 6. (indent)
src/embeds/ProfileEmbed.js
Outdated
| value: player.clan.type ? `${player.clan.name}\nRank **${player.clan.rank}** ${player.clan.type}` : player.clan.name, | ||
| inline: true, | ||
| }, | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 8 spaces but found 6. (indent)
src/embeds/ProfileEmbed.js
Outdated
| inline: true, | ||
| }, | ||
| { | ||
| name: 'Marked for Death', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 10 spaces but found 8. (indent)
src/embeds/ProfileEmbed.js
Outdated
| }, | ||
| { | ||
| name: 'Marked for Death', | ||
| value: (player.marked.stalker || player.marked.g3 || player.marked.zanuka) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 10 spaces but found 8. (indent)
src/embeds/ProfileEmbed.js
Outdated
| inline: true, | ||
| }); | ||
| : 'Unmarked', | ||
| inline: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 10 spaces but found 8. (indent)
src/embeds/ProfileEmbed.js
Outdated
| }); | ||
| : 'Unmarked', | ||
| inline: true, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 8 spaces but found 6. (indent)
| * Raven client for logging errors and debugging events | ||
| * @type {Raven} | ||
| */ | ||
| const NexusFetcher = require('nexus-stats-api'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unable to resolve path to module 'nexus-stats-api'. (import/no-unresolved)
| * Raven client for logging errors and debugging events | ||
| * @type {Raven} | ||
| */ | ||
| const NexusFetcher = require('nexus-stats-api'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unable to resolve path to module 'nexus-stats-api'. (import/no-unresolved)
another attemp to fix false positive (this is only happening on stickler-ci, so no idea why)
|
merging. |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Moved basic error logging to main.js as global exception handler. (no more
.catch(this.logger.error))Made use of ES7
asyncin the main modules for now. I recommend testing this, before starting with commands. (Will append commits to this PR)