-
-
Notifications
You must be signed in to change notification settings - Fork 520
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
Refactor/fix clippy errors #2056
Refactor/fix clippy errors #2056
Conversation
clippy.toml
Outdated
@@ -0,0 +1 @@ | |||
allow-unwrap-in-tests = true |
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 have added this file to configure clippy.
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.
May be a #![allow(clippy::unwrap_used, clippy::expect_used)]
directive in test files would be better?
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 do that. It might be better because clippy config file like clippy.toml
is unstable.
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.
Fixed
889d250
@@ -345,7 +345,6 @@ pub mod tests_cfg; | |||
mod util; | |||
|
|||
pub use database::*; | |||
pub use driver::*; |
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.
Instead of removing this line, I think we should add #[allow(clippy::unused_xxx)]
, because the content of the re-export depends on feature flags
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.
Fixed
138b9dc
I will add #[allow(clippy::unused_xxx)]
later because I could not see clippy error in my local somehow...
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 have added #[allow(unused_imports)]
20dc881
@@ -2,7 +2,6 @@ use crate::{ | |||
ColumnTrait, EntityTrait, IdenStatic, Iterable, QueryTrait, Select, SelectTwo, SelectTwoMany, | |||
}; | |||
use core::marker::PhantomData; | |||
pub use sea_query::JoinType; |
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.
can confirm this is already exported in query.rs
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.
Oh, it is not exported so I have reverted this change. e4f4a12
@@ -15,7 +15,6 @@ pub use combine::{SelectA, SelectB}; | |||
pub use delete::*; | |||
pub use helper::*; | |||
pub use insert::*; | |||
pub use join::*; |
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.
The file contains only impl, so nothing to export
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.
In that case, do we need to keep pub use join::*;
?
src/driver/sqlx_mysql.rs
Outdated
@@ -36,6 +36,7 @@ impl std::fmt::Debug for SqlxMySqlPoolConnection { | |||
|
|||
impl SqlxMySqlConnector { | |||
/// Check if the URI provided corresponds to `mysql://` for a MySQL database | |||
#[cfg(feature = "mock")] |
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.
This is what confuses me, this accepts
method should be in the public API, why does clippy think it's redundant?
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 think that is because the accepts
method is not used from anywhere in this workspace except for mock
. But if we want this method to be public, we need #[allow(unused_variables)]
instead so I will replace them.
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.
Remove feature flag and I put #[allow(unused_variables)]
instead
09f845a
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.
Thank you. I will make some tweaks
Thank you so much for your review. I will check your PR and would like to learn from it:) |
* chore: add clippy config file * refactor: fix clippy errors and wornings of runtime-async-std-native-tls,sqlx-all * refactor: fix clippy errors and wornings of sqlx-sqlite, sqlx-mysql, sqlx-postgres * chore: format * refactor: fix clippy * fix: import path * refactor: fix clippy errors and wornings of sqlx-sqlite, sqlx-mysql, sqlx-postgres * fix: revert some space and comma removal * fix: revert some space and comma removal * refactor: add feature flag * fix: import path * test: remove mismatch feature flag * test: remove mismatch feature flag * chore: add proper feature flag * chore: remove feature flag * refactor: remove clippy.toml file * fix: re-export driver * fix: re-export JoinType * fix: remove feature flag * chore: add #[allow(unused_imports)] for driver
* chore: add clippy config file * refactor: fix clippy errors and wornings of runtime-async-std-native-tls,sqlx-all * refactor: fix clippy errors and wornings of sqlx-sqlite, sqlx-mysql, sqlx-postgres * chore: format * refactor: fix clippy * fix: import path * refactor: fix clippy errors and wornings of sqlx-sqlite, sqlx-mysql, sqlx-postgres * fix: revert some space and comma removal * fix: revert some space and comma removal * refactor: add feature flag * fix: import path * test: remove mismatch feature flag * test: remove mismatch feature flag * chore: add proper feature flag * chore: remove feature flag * refactor: remove clippy.toml file * fix: re-export driver * fix: re-export JoinType * fix: remove feature flag * chore: add #[allow(unused_imports)] for driver Co-authored-by: Shogo Nakano <61229807+shogo-nakano-desu@users.noreply.github.com>
Changes
Fix clippy errors and warnings. This is a follow up PR of #2030