Copilot commented on code in PR #17:
URL: https://github.com/apache/sedona-db/pull/17#discussion_r2330580124
##########
c/sedona-s2geography/build.rs:
##########
@@ -62,23 +77,28 @@ fn main() {
fn parse_cmake_linker_flags(binary_dir: &Path) {
// e.g., libabsl_base.a
- let re_lib = Regex::new("^(lib|)([^.]+).*?(lib|a|dylib|so|dll)$").unwrap();
+ let re_lib =
Regex::new("^(lib|)([^.]+).*?(LIB|lib|a|dylib|so|dll)$").unwrap();
Review Comment:
[nitpick] The regex pattern includes both 'LIB' and 'lib' which could lead
to confusion. Consider using a case-insensitive match or documenting why both
cases are needed for clarity.
```suggestion
let re_lib =
Regex::new("(?i)^(lib|)([^.]+).*?(lib|a|dylib|so|dll)$").unwrap();
```
##########
rust/sedona-functions/src/st_setsrid.rs:
##########
@@ -214,13 +214,8 @@ mod test {
assert_value_equal(&result, &geom_lnglat);
// Call with an integer code of 0 (should unset the output crs)
- let (return_type, result) = call_udf(
- &udf,
- geom_lnglat.clone(),
- WKB_GEOMETRY,
- unset_scalar.clone(),
- )
- .unwrap();
+ let (return_type, result) =
+ call_udf(&udf, geom_lnglat.clone(), wkb_lnglat,
unset_scalar.clone()).unwrap();
Review Comment:
This refactoring changes the function call signature but appears to be
inconsistent with the pattern used above. The previous call uses `WKB_GEOMETRY`
as the third parameter, but this uses `wkb_lnglat`. Consider verifying this is
the intended type or maintaining consistency with the pattern above.
```suggestion
call_udf(&udf, geom_lnglat.clone(), WKB_GEOMETRY,
unset_scalar.clone()).unwrap();
```
--
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]
