-
Notifications
You must be signed in to change notification settings - Fork 31
Conversation
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.
Various changes just to stay with optimisation as well as coding conventions
@@ -11,7 +11,9 @@ exports.init = (client) => { | |||
// Load default configuration, create if not exist. | |||
defaultConf = { | |||
prefix: {type: "String", data: client.config.prefix}, | |||
disabledCommands: {type: "Array", data: "[]"} | |||
disabledCommands: {type: "Array", data: []}, | |||
mod_role: {type: "String", data: "Mods"}, |
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.
Instead of hard coding Mods and Devs to be default
Why not default them as blank values?
disabledCommands: {type: "Array", data: "[]"} | ||
disabledCommands: {type: "Array", data: []}, | ||
mod_role: {type: "String", data: "Mods"}, | ||
admin_role: {type: "String", data: "Devs"} |
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.
Same thing with Line 16
let member = guild.member(user); | ||
let mod_role = guild.roles.find("name", "Mods"); | ||
let mod_role = guild.roles.find("name", guildConf.mod_role); |
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.
Also like please use camelCase for modRole
if (mod_role && member.roles.has(mod_role.id)) | ||
permlvl = 2; | ||
let admin_role = guild.roles.find("name", "Devs"); | ||
let admin_role = guild.roles.find("name", guildConf.admin_role); |
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.
Same thing here
@CyberiumShadow For the last 2 comments, followed snake case in the original Komada permissionLevel.js, and for why not defaulting them to undefined you could do that. but then whats the point of having a default configuration? It'd be like defaulting the prefix to "". Just my person opinion though. |
Fair enough |
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.
Well, everything seems good @eslachance
Adds support for custom roles per server. Also fixes the disabledCommands to be an actual array as opposed to the string "[]". Made the custom roles two different strings so they could easily be edited with the conf core command, instead of dumping them into an object.