Skip to content

Make clippy happy #140

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

Merged
merged 1 commit into from
Jul 15, 2022
Merged

Make clippy happy #140

merged 1 commit into from
Jul 15, 2022

Conversation

margold
Copy link
Contributor

@margold margold commented Jul 13, 2022

Clippy is failing on master, I think due to a rust update? Attempting to fix.

@margold margold requested a review from dan-ri July 13, 2022 17:08
@@ -80,14 +81,18 @@ where
{
use std::ops::Deref;
let mut template_str = String::new();
template_str.push_str(&format!("{} ", LOG_PREFIX_INFO.deref()));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was the specific lint: https://rust-lang.github.io/rust-clippy/master/index.html#format_push_string
not sure if this is the best way to fix it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lint is problematic because of the way the proposed fix introduces fallibility (see discussion and restriction). Still, this change should be fine here.

@@ -80,14 +81,18 @@ where
{
use std::ops::Deref;
let mut template_str = String::new();
template_str.push_str(&format!("{} ", LOG_PREFIX_INFO.deref()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lint is problematic because of the way the proposed fix introduces fallibility (see discussion and restriction). Still, this change should be fine here.

@margold margold merged commit 7918af5 into master Jul 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants