feat(model,http,http-ratelimiting): Add support for application role connections metadata get/set operations #2004
Conversation
github-actions
bot
added
c-http
MOZGIII
changed the title
MOZGIII
force-pushed
the
role-connections
branch
from
604bc63
to
0f4be11
Compare
MOZGIII
force-pushed
the
role-connections
branch
from
0f4be11
to
e4494f4
Compare
| @@ -1,2 +1,3 @@ | |||
| pub mod command; | |||
| pub mod interaction; | |||
| pub mod role_connection; | |||
There was a problem hiding this comment.
I'd rather place this into guild.
| @@ -0,0 +1,54 @@ | |||
| //! Application role connections models. | |||
There was a problem hiding this comment.
I'd prefer a more descriptive module description. Something summarizing what and why role connections exists would be ideal.
There was a problem hiding this comment.
This is largely a wip at the moment, the docs are either placeholders or taken straight out of the discord's docs (the uncapitalized ones).
When it is deemed this PR makes sense I'll put more effort into correcting the docs, but for now, it's a bit premature.
| pub key: String, | ||
| /// name of the metadata field (max 100 characters) | ||
| pub name: String, | ||
| /// translations of the name |
There was a problem hiding this comment.
Could you update the localization items' documentation to follow the same style as CommandOption?
| @@ -1,2 +1,3 @@ | |||
| pub mod command; | |||
| pub mod interaction; | |||
| pub mod role_connections; | |||
There was a problem hiding this comment.
As with in twligiht-model, I'd rather have this in guild.
There was a problem hiding this comment.
I'm not sure what you're referring to. Just to be clear - this is the app-global part of the API, not the user-specific (or guild-specific) one.
| mod get_metadata; | ||
| mod set_metadata; | ||
|
|
||
| pub use self::{get_metadata::GetMetadata, set_metadata::SetMetadata}; |
There was a problem hiding this comment.
Naming could be improved, GetRoleConnection and UpdateRoleConnection sounds a bit better and more futureproof to me.
There was a problem hiding this comment.
True! However, these ones are for configuring and reading the metadata for the role connections that app provides. There also has be an API for setting the actual values of the role connections per user (the @me API).
| @@ -276,6 +278,14 @@ impl Client { | |||
| InteractionClient::new(self, application_id) | |||
| } | |||
|
|
|||
| /// Create an interface for using application role connections. | |||
| pub const fn role_connections( | |||
There was a problem hiding this comment.
This is unacceptable (not your fault), it's way to contrived. We need a RFC for how to handle routes requiring an application_id.
There was a problem hiding this comment.
That's fine! I'll use this for now in my code as I need something that works immediately. I'll be looking forward to the RFC and the proper implementation of this feature but it is unlikely I'll be taking part in the RFC process itself in the near future.
vilgotf
added
the
w-unapproved
|
Great feedback, thank you @vilgotf! |
|
|
|
Sure! Feel free to take over, if needed - fork my branch and create a new one. I'd have no problem with that whatsoever. |
To do:
Refs: