Enhance/create role #323
Enhance/create role #323
Conversation
lib/discordrb/data.rb
Outdated
| @@ -2479,9 +2479,17 @@ def create_channel(name, type = 0) | |||
| # Creates a role on this server which can then be modified. It will be initialized (on Discord's side) | |||
| # with the regular role defaults the client uses, i. e. name is "new role", permissions are the default, | |||
| # colour is the default etc. | |||
| # If all arguments are provided it will create the role with custom name, colour, hoist, mentionable, | |||
| # and packed_permissions properties, if they are not all provided it will just create the role with | |||
| # default settings. | |||
There was a problem hiding this comment.
Please document this properly with the @overload syntax
You can see an example here: https://github.com/meew0/discordrb/blob/master/lib/discordrb/data.rb#L174-L181
lib/discordrb/data.rb
Outdated
| API::Server.create_role(@bot.token, @id) | ||
| else | ||
| API::Server.create_role(@bot.token, @id, name, colour, hoist, mentionable, packed_permissions) | ||
| end |
There was a problem hiding this comment.
I'm not sure how I feel about this -- while using ordered parameters is fine, I think these should be named parameters as the official documentation says that all parameters are optional.
Perhaps we could also take a route akin to what Server#update_server_data, which is to pull from the current settings (or in this case, as this is creating a new Role, we pull from the @everyone role). I don't know, I just don't really like this being all-or-none in terms of parameters.
There was a problem hiding this comment.
I suggested that Reaver do this in the interest of maintaining the existing functionality without having to maintain defaults in two places.
I must have missed that all parameters are optional ; I otherwise didn't see what any defaults would have been provided at app in API in the first place.
If that's the case, then we can do as you suggested and drop the if fork entirely.
There was a problem hiding this comment.
I'd prefer if the defaults were set for this method instead of the API one, since this one will get used much more. It's true that some other API methods have defaults set but they're not really very useful. Changing it to this one also allows getting rid of this check, making the code cleaner.
lib/discordrb/data.rb
Outdated
| API::Server.create_role(@bot.token, @id) | ||
| else | ||
| API::Server.create_role(@bot.token, @id, name, colour, hoist, mentionable, packed_permissions) | ||
| end |
There was a problem hiding this comment.
I'd prefer if the defaults were set for this method instead of the API one, since this one will get used much more. It's true that some other API methods have defaults set but they're not really very useful. Changing it to this one also allows getting rid of this check, making the code cleaner.
meew0
added
api-support
#309 API::Server.create_role, missing name, permissions, color, hoist, mentionable parameters
Adds those missing parameters to the create_role method.
If all parameters are sent it will create it with those set parameters, if not it will fallback to defaults.