paleolimbot commented on code in PR #227:
URL: https://github.com/apache/sedona-db/pull/227#discussion_r2440396563
##########
rust/sedona-geo-generic-alg/src/algorithm/area.rs:
##########
Review Comment:
The files in sedona-geo-generic-alg are mostly copied from another source. I
think moving around the imports may make it difficult to track updates to the
upstream code.
##########
rust/sedona-geoparquet/src/metadata.rs:
##########
@@ -72,7 +74,6 @@ impl Default for GeoParquetColumnEncoding {
impl Display for GeoParquetColumnEncoding {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
- use GeoParquetColumnEncoding::*;
Review Comment:
This one should also probably stay here (it is a wildcard import of an enum
that brings a lot of names into scope)
##########
rust/sedona-functions/src/barrier.rs:
##########
@@ -332,8 +333,6 @@ impl Barrier {
/// Compare two scalar values using the given operator
fn compare_values(left: &ScalarValue, op: &str, right: &ScalarValue) ->
Result<bool> {
- use ScalarValue::*;
-
Review Comment:
This should probably stay here...the wildcard imports from enums are often
best kept local because they may bring a lot of common names into scope!
##########
rust/sedona/src/object_storage.rs:
##########
@@ -819,8 +821,6 @@ mod tests {
#[cfg(not(target_os = "windows"))]
#[test]
fn test_substitute_tilde() {
- use std::env;
- use std::path::MAIN_SEPARATOR;
Review Comment:
These should probably stay here: function level imports under a `[cfg(...)]`
help prevent clippy errors about unused imports when compiling on a different
platform.
--
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]
