chore: fix some compiler warnings #3262
Conversation
alamb
requested review from
wjones127,
roeap,
rtyler,
hntd187 and
ion-elgreco
as code owners
| @@ -41,6 +41,7 @@ impl AWSForObjectStore { | |||
| } | |||
|
|
|||
| /// Return true if a credential has been cached | |||
| #[cfg(test)] | |||
| async fn has_cached_credentials(&self) -> bool { | |||
There was a problem hiding this comment.
this is only used in tests it seems
|
ACTION NEEDED delta-rs follows the Conventional Commits specification for release automation. The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification. |
| struct ScopedEnv { | ||
| vars: HashMap<std::ffi::OsString, std::ffi::OsString>, | ||
| } | ||
| struct ScopedEnv {} |
There was a problem hiding this comment.
the compiler claimed this field was never read -- if the CI passes we could remove the entire structure I think (but I wanted to make the diff smaller to see initially)
alamb
force-pushed
the
alamb/fix_warnings2
branch
from
811f579 to
2d8043f
Compare
|
I will make another follow on PR for clippy errors I am seeing locally if this one passes |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3262 +/- ##
=======================================
Coverage 72.20% 72.21%
=======================================
Files 143 143
Lines 45607 45610 +3
Branches 45607 45610 +3
=======================================
+ Hits 32932 32938 +6
- Misses 10591 10598 +7
+ Partials 2084 2074 -10 ☔ View full report in Codecov by Sentry. |
alamb
changed the title
alamb
force-pushed
the
alamb/fix_warnings2
branch
from
2d8043f to
f7a6d69
Compare
| @@ -145,17 +145,6 @@ static REMOVE_FIELD: LazyLock<StructField> = LazyLock::new(|| { | |||
| true, | |||
| ) | |||
| }); | |||
| static REMOVE_FIELD_CHECKPOINT: LazyLock<StructField> = LazyLock::new(|| { | |||
There was a problem hiding this comment.
I think we want to keep this around for now
There was a problem hiding this comment.
The compiler says it is unused. I can fix the compiler warning by adding an #allow(unused)
Can you give me some hint about why we are keeping it around so I can add a comment for the next person?
There was a problem hiding this comment.
I think this is part of the checkpoint we still need to support at some point
| @@ -79,15 +79,6 @@ where | |||
| serializer.serialize_str(&json_string) | |||
| } | |||
|
|
|||
| // Custom deserialization that parses a JSON string into MetricDetails | |||
| fn deserialize_vec_string<'de, D>(deserializer: D) -> Result<Vec<String>, D::Error> | |||
There was a problem hiding this comment.
The compiler says it is unused (and the function isn't pub so it can't be called outside this module). I will put it back with an #allow(unused)
Any chance you can give me the rationale to add to the comments for future readers?
alamb
force-pushed
the
alamb/fix_warnings2
branch
from
2e0fd8e to
2dc030f
Compare
|
Fixed |
Signed-off-by: Andrew Lamb <[email protected]>
Signed-off-by: Andrew Lamb <[email protected]>
ion-elgreco
force-pushed
the
alamb/fix_warnings2
branch
from
2dc030f to
72a8a9c
Compare
Description
chore: fix compiler warnings
While working on #3261 I found several items that generated warnings locally for me:
For example:
I looked at the CI tests and they seem to be passing, so I am not sure why this fails locally for me
Related Issue(s)
46.0.0apache/datafusion#14123Documentation