Copilot commented on code in PR #95:
URL: https://github.com/apache/fluss-rust/pull/95#discussion_r2617029430
##########
crates/fluss/tests/integration/table.rs:
##########
@@ -16,17 +16,17 @@
* limitations under the License.
*/
-use once_cell::sync::Lazy;
use parking_lot::RwLock;
use std::sync::Arc;
+use std::sync::LazyLock;
Review Comment:
The std::sync imports are split across multiple lines (Arc and LazyLock on
separate lines). Consider consolidating these imports into a single line for
consistency: `use std::sync::{Arc, LazyLock};`
```suggestion
use std::sync::{Arc, LazyLock};
```
##########
crates/fluss/tests/integration/admin.rs:
##########
@@ -16,16 +16,17 @@
// under the License.
use crate::integration::fluss_cluster::FlussTestingCluster;
-use once_cell::sync::Lazy;
use parking_lot::RwLock;
+
use std::sync::Arc;
+use std::sync::LazyLock;
Review Comment:
The std::sync imports are split across multiple lines (Arc and LazyLock on
separate lines). While this is valid and accepted by Rust, the other test files
(table.rs and table_remote_scan.rs) follow a similar pattern. Consider
consolidating these imports into a single line for consistency: `use
std::sync::{Arc, LazyLock};`
```suggestion
use std::sync::{Arc, LazyLock};
```
##########
crates/fluss/src/row/datum.rs:
##########
@@ -414,16 +412,17 @@ impl Date {
self.0
}
- pub fn year(&self) -> i32 {
- let date = NaiveDate::from_num_days_from_ce_opt(self.0 +
UNIX_EPOCH_DAYS).unwrap();
+ pub fn year(&self) -> i16 {
+ let date = UNIX_EPOCH_DAY + self.0.days();
date.year()
}
- pub fn month(&self) -> i32 {
- let date = NaiveDate::from_num_days_from_ce_opt(self.0 +
UNIX_EPOCH_DAYS).unwrap();
- date.month() as i32
+ pub fn month(&self) -> i8 {
+ let date = UNIX_EPOCH_DAY + self.0.days();
+ date.month()
}
- pub fn day(&self) -> i32 {
- let date = NaiveDate::from_num_days_from_ce_opt(self.0 +
UNIX_EPOCH_DAYS).unwrap();
- date.day() as i32
+
+ pub fn day(&self) -> i8 {
+ let date = UNIX_EPOCH_DAY + self.0.days();
+ date.day()
}
Review Comment:
The return types for year(), month(), and day() methods have been changed
from i32/i32/i32 to i16/i8/i8 respectively. This is a breaking API change that
could affect external consumers of these public methods. Consider documenting
this breaking change in the PR description or changelog, or verify that these
methods are not part of the public API surface that external crates depend on.
##########
crates/fluss/tests/integration/table_remote_scan.rs:
##########
@@ -16,16 +16,16 @@
* limitations under the License.
*/
use crate::integration::fluss_cluster::FlussTestingCluster;
-use once_cell::sync::Lazy;
use parking_lot::RwLock;
use std::sync::Arc;
+use std::sync::LazyLock;
Review Comment:
The std::sync imports are split across multiple lines (Arc and LazyLock on
separate lines). Consider consolidating these imports into a single line for
consistency: `use std::sync::{Arc, LazyLock};`
```suggestion
use std::sync::{Arc, LazyLock};
```
--
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]
