Phoenix500526 commented on code in PR #11184:
URL: https://github.com/apache/arrow-rs/pull/11184#discussion_r4173331241
##########
parquet/tests/arrow_writer/roundtrip_helpers.rs:
##########
@@ -127,39 +109,12 @@ impl RoundTripTest {
}
}
- /// Set the schema
- pub(super) fn with_schema(mut self, schema: SchemaRef) -> Self {
- self.schema = Some(schema);
- self
- }
-
/// Set the nullable flag
pub(super) fn with_nullable(mut self, nullable: bool) -> Self {
self.nullable = nullable;
self
}
- /// Set bloom filter
- pub(super) fn with_bloom_filter(mut self, bloom_filter: bool) -> Self {
- self.bloom_filter = bloom_filter;
- self
- }
-
- /// Set bloom filter max ndv
- pub(super) fn with_bloom_filter_ndv(mut self, bloom_filter_ndv: u64) ->
Self {
- self.bloom_filter_ndv = Some(bloom_filter_ndv);
Review Comment:
I tried putting #[expect(dead_code)] on the individual methods, but that
moves the warning to the other test suite. For example, with_schema is unused
in unit tests, so the expectation is satisfied there. In integration tests it
is used, so Rust reports an unfulfilled_lint_expectations warning instead. The
Bloom filter methods have the opposite problem.
Using #[allow(dead_code)] handles both cases, but conflicts with the
project's clippy::allow_attributes lint and requires another exception.
Given that, I think the current split may be a better fit: keep the shared
implementation centralized, and define the suite-specific methods alongside
their callers. This avoids both module-wide suppression and additional lint
exceptions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
