-
Notifications
You must be signed in to change notification settings - Fork 17
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
Tidy/minor code changes #118
Conversation
- move quota_project_id lookup to type declaration location in order to make control flow cleaner - use more functional, less verbose syntax
)) | ||
})?; | ||
match &url_creds.format { | ||
None | Some(ExternalCredentialUrlFormat::Text) => Ok(response.text().await?.into()), |
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.
We can merge None
case and Some(Text)
case in one clause.
})?; | ||
let json_object = json.as_object().ok_or_else(|| { | ||
match &url_creds.format { | ||
None | Some(ExternalCredentialUrlFormat::Text) => Ok(file_content.into()), |
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.
@@ -18,6 +18,16 @@ pub enum Credentials { | |||
ExternalAccount(ExternalAccount), | |||
} | |||
|
|||
impl Credentials { | |||
pub(crate) fn quota_project_id(&self) -> Option<&str> { |
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.
I usually put this kind of field access in struct impl section.
@@ -166,7 +176,7 @@ mod jwt { | |||
} | |||
} | |||
|
|||
#[derive(Debug, serde::Serialize, serde::Deserialize)] |
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.
No need to derive Deserialize.
remove unused derives from private structs and enums
03cb9d2
to
784930b
Compare
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.
Thanks for clean ups!
There is still some old verbose code around token sources particularly that I clean from time to time myself.
follow up abdolence#118
* tidy: remove duplicated code follow up #118 * chore: fix typo
This commit changes some codes that I found are a bit verbose and complex without changing their logic.