diff --git a/src/instance.rs b/src/instance.rs index f151ced..64b33ce 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -112,8 +112,9 @@ impl Instance { /// Creates a new [`Instance`] by trying to get the [relevant instance urls](UrlBundle) from a root url. /// Shorthand for `Instance::new(UrlBundle::from_root_domain(root_domain).await?)`. - /// - /// If `limited` is `true`, then Chorus will track and enforce rate limits for this instance. + // RAGC: Can we really call this a conversion? + // Would with_root_url be better? Not really I think, because with is for more details + // Where are this is with.. less? (or rather with other ones) pub async fn from_root_url(root_url: &str) -> ChorusResult { let urls = UrlBundle::from_root_url(root_url).await?; Instance::new(urls).await diff --git a/src/types/entities/attachment.rs b/src/types/entities/attachment.rs index ffbc520..0fb3e46 100644 --- a/src/types/entities/attachment.rs +++ b/src/types/entities/attachment.rs @@ -57,6 +57,7 @@ pub struct PartialDiscordFileAttachment { } impl PartialDiscordFileAttachment { + // RAGC: Is move_x proper naming? /// Moves `self.content` out of `self` and returns it. pub fn move_content(self) -> (Vec, PartialDiscordFileAttachment) { let content = self.content; diff --git a/src/types/entities/config.rs b/src/types/entities/config.rs index 25b1ef1..e09d5c4 100644 --- a/src/types/entities/config.rs +++ b/src/types/entities/config.rs @@ -11,13 +11,21 @@ pub struct ConfigEntity { } impl ConfigEntity { - pub fn as_string(&self) -> Option { + // RAGC: not sure about this, but it performs an "expensive" to_string opeartion, resulting in + // "borrowed -> owned" ownership + pub fn to_string(&self) -> Option { let Some(v) = self.value.as_ref() else { return None; }; Some(v.as_str().expect("value is not a string").to_string()) } + // RAGC: Is this proper naming? + // If you check https://rust-lang.github.io/api-guidelines/naming.html#c-conv + // + // as_* should be "borrowed -> borrowed" ownership; + // This has "borrowed -> owned" ownership, yet isn't a to_*, because it isn't expensive. + // It seems the inner serde type has the same issue, so I am happy to just leave this be pub fn as_bool(&self) -> Option { let Some(v) = self.value.as_ref() else { return None; diff --git a/src/types/entities/user.rs b/src/types/entities/user.rs index a7bdf63..d9f70ce 100644 --- a/src/types/entities/user.rs +++ b/src/types/entities/user.rs @@ -26,7 +26,7 @@ pub struct UserData { } impl User { - pub fn to_public_user(self) -> PublicUser { + pub fn into_public_user(self) -> PublicUser { PublicUser::from(self) } } diff --git a/src/types/utils/rights.rs b/src/types/utils/rights.rs index 5a3c373..73ae5f7 100644 --- a/src/types/utils/rights.rs +++ b/src/types/utils/rights.rs @@ -124,6 +124,7 @@ bitflags! { } impl Rights { + // FIXME: Why are any and has the same?? pub fn any(&self, permission: Rights, check_operator: bool) -> bool { (check_operator && self.contains(Rights::OPERATOR)) || self.contains(permission) } @@ -138,6 +139,9 @@ impl Rights { /// # Notes /// Unlike has, this returns an Error if we are missing rights /// and Ok(true) otherwise + // RAGC: Is this proper naming? + // I don't think it is mentioned anywhere how this should be named + // Also, isn't it redundant to return a bool, if it's always going to be true? pub fn has_throw(&self, permission: Rights) -> Result { if self.has(permission, true) { Ok(true)