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

refactor command permission check #538

Conversation

alex-taxiera
Copy link
Contributor

Fixes #537

Copy link
Contributor

@eritbh eritbh left a comment

Choose a reason for hiding this comment

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

Nothing critical, just a couple things I noticed to be considered.

lib/command/Command.js Outdated Show resolved Hide resolved
if(this.requirements.custom(msg)) {
return true;
}
if(this.requirements.custom && typeof this.requirements.custom === "function" && !this.requirements.custom(msg)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not add await to the function calls to add support for async functions? Would require a docs update. This also applies to a bunch of other things that can be functions rather than plain values. Not required, just something to consider.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

working on it

@abalabahaha abalabahaha merged commit 07d24f5 into abalabahaha:dev Oct 13, 2019
@alex-taxiera alex-taxiera deleted the bug/537-command-client-permission-check branch June 16, 2020 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants