-
Notifications
You must be signed in to change notification settings - Fork 155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: minimal versions check #1237
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can reproduce the error with the latest commit, create a hello-world crate and add the latest Openraft as its dependency, then:
$ cd hello-world
$ cargo minimal-versions check
error[E0599]: no associated item named `ZERO` found for struct `rust_decimal::Decimal` in the current scope
--> /home/steve/.cargo/registry/src/index.crates.io-6f17d22bba15001f/byte-unit-5.1.4/src/byte/decimal.rs:27:29
|
27 | if size >= Decimal::ZERO {
| ^^^^ associated item not found in `Decimal`
error[E0277]: the trait bound `rust_decimal::Decimal: From<u128>` is not satisfied
--> /home/steve/.cargo/registry/src/index.crates.io-6f17d22bba15001f/byte-unit-5.1.4/src/byte/decimal.rs:73:45
|
73 | _ => match size.checked_mul(Decimal::from(unit.as_bytes_u128())) {
| ^^^^^^^ the trait `From<u128>` is not implemented for `rust_decimal::Decimal`
...
As we can see, the errors originate from the byte_unit
crate, then I tried:
$ git clone https://github.com/magiclen/Byte-Unit.git
$ cd Byte-Unit
$ cargo minimal-versions check
rror[E0599]: no associated item named `ZERO` found for struct `rust_decimal::Decimal` in the current scope
--> src/byte/decimal.rs:27:29
|
27 | if size >= Decimal::ZERO {
| ^^^^ associated item not found in `Decimal`
error[E0277]: the trait bound `rust_decimal::Decimal: From<u128>` is not satisfied
--> src/byte/decimal.rs:73:45
|
73 | _ => match size.checked_mul(Decimal::from(unit.as_bytes_u128())) {
| ^^^^^^^ the trait `From<u128>` is not implemented for `rust_decimal::Decimal`
|
I got the same error, so I kinda think we should fix this in byte_unit
, not Openraft🤔
Reviewable status: 0 of 2 files reviewed, all discussions resolved
Oh yes, that would make sense as well, of course.
Edit: Opened a PR over there. |
…inimal-versions # Conflicts: # Cargo.toml
@sebadob This issue stems from a dependency crate rather than our crate directly. Introducing a new unnecessary crate solely to fix this issue may not be the most elegant solution. If the minimal-versions check is critical for your application, a more appropriate solution would be adding |
Yes it's coming from a dependency, but it's not being added, it's there anyway and nothing additional. Checking for minimal versions is a good practice for any crate. This example may not be the best one, because the issue has been introduced in a version 3 years ago, but the same happens for new releases as well (sadly, quite often), which can lead to very weird errors, when you are stuck on an older version in your application for whatever reason. It's fine for me to just close the issue then. |
Yeah, true, perhaps we can have this check in Openraft's CI as well:) |
Running it on in Openraft root dir yields an error. I'm not sure if it is a bug of minimal-versions parsing
And running
And |
Yes it does. It basically uses the Edit: Found the issue. It was because error[E0599]: no method named `timestamp_nanos_opt` found for struct `DateTime` in the current scope
--> openraft/src/metrics/serde_instant.rs:102:33
|
102 | let nano = datetime.timestamp_nanos_opt().ok_or(serde::ser::Error::custom("time out of range"))?;
| ^^^^^^^^^^^^^^^^^^^ method not found in `DateTime<Utc>`
error[E0599]: no function or associated item named `from_timestamp_nanos` found for struct `DateTime` in the current scope
--> openraft/src/metrics/serde_instant.rs:124:46
|
124 | let datetime = DateTime::from_timestamp_nanos(value as i64);
| ^^^^^^^^^^^^^^^^^^^^ function or associated item not found in `DateTime<_>`
|
note: if you're trying to build a new `DateTime<_>` consider using one of the following associated functions:
DateTime::<Tz>::from_utc
DateTime::<FixedOffset>::parse_from_rfc2822
DateTime::<FixedOffset>::parse_from_rfc3339
DateTime::<FixedOffset>::parse_from_str
--> /home/sd/.cargo/registry/src/index.crates.io-6f17d22bba15001f/chrono-0.4.0/src/datetime.rs:71:5
|
71 | pub fn from_utc(datetime: NaiveDateTime, offset: Tz::Offset) -> DateTime<Tz> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
191 | pub fn parse_from_rfc2822(s: &str) -> ParseResult<DateTime<FixedOffset>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
203 | pub fn parse_from_rfc3339(s: &str) -> ParseResult<DateTime<FixedOffset>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
216 | pub fn parse_from_str(s: &str, fmt: &str) -> ParseResult<DateTime<FixedOffset>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ |
If |
Checklist
This is a tiny fix for minimal versions. I stumbled about this issue when testing with the latest
openraft:0.9.15
when executing in my pipeline:This change is