Skip to content
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

Use chrono instead of time. #199

Merged
merged 1 commit into from
Jul 30, 2024
Merged

Use chrono instead of time. #199

merged 1 commit into from
Jul 30, 2024

Conversation

01mf02
Copy link
Owner

@01mf02 01mf02 commented Jul 30, 2024

The crate time does not build on Rust 1.80 for versions smaller than 0.3.36: time-rs/time#681
Apart from the fact that I do not like packages stopping to build when updating the compiler, updating time to 0.3.36 would require raising MSRV to 1.67, which I do not like.

I found that the chrono crate has a lower MSRV, it works fine on all compiler versions I tried, and using it is considerably simpler for my use case, so this PR replaces time with chrono.
A small side effect of this is that jaq outputs fractional seconds with 6 digits instead of 9 before.

@01mf02 01mf02 merged commit 94d1135 into main Jul 30, 2024
2 checks passed
@01mf02 01mf02 deleted the chrono branch July 30, 2024 07:10
@jhpratt
Copy link

jhpratt commented Aug 1, 2024

Apart from the fact that I do not like packages stopping to build when updating the compiler

This is not the fault of time.

updating time to 0.3.36 would require raising MSRV to 1.67, which I do not like.

Why do you need an MSRV that is more than 18 months old? Very few crates have policies that extend that far back (with chrono not having a policy at all, mind you).

@01mf02
Copy link
Owner Author

01mf02 commented Aug 2, 2024

Apart from the fact that I do not like packages stopping to build when updating the compiler

This is not the fault of time.

I'm sorry, I was indeed under the impression that this was the fault of time. Rereading your post (time-rs/time#681 (comment)), I see that this does not seem to be the case after all. I hope that you accept my apology.
Still, I do not understand how adding a new trait implementation to the standard library could break your code. I thought that Rust's coherence rules prevent such breakage? If you can enlighten me on this point, I would be very interested.

updating time to 0.3.36 would require raising MSRV to 1.67, which I do not like.

Why do you need an MSRV that is more than 18 months old? Very few crates have policies that extend that far back (with chrono not having a policy at all, mind you).

A colleague of mine who was not into Rust at that time once tried to install one of my Rust programs on Debian and failed because his system-installed compiler was too old. I just checked now, and the rustc shipped with stable Debian is 1.63. So I'm trying to preserve MSRV as long as it is not too much effort.

@jhpratt
Copy link

jhpratt commented Aug 3, 2024

Still, I do not understand how adding a new trait implementation to the standard library could break your code. I thought that Rust's coherence rules prevent such breakage? If you can enlighten me on this point, I would be very interested.

Essentially, the code I had written relied on the fact that the return type of .into() could be inferred. Type inference is something explicitly permitted as breakage. With the addition of IntoIterator for Box<[T]>, this was no longer the case. This specific instance of breakage was caught months ago, which is why a patch was merged and released in April.

A colleague of mine who was not into Rust at that time once tried to install one of my Rust programs on Debian and failed because his system-installed compiler was too old.

I honestly don't understand why they even bother shipping a version so old (it was released two full years ago). rustup is always the recommended way to do anything related to Rust, in my opinion.

@01mf02
Copy link
Owner Author

01mf02 commented Aug 5, 2024

I honestly don't understand why they even bother shipping a version so old (it was released two full years ago). rustup is always the recommended way to do anything related to Rust, in my opinion.

I also use rustup, but from my own experience with people who are just trying Rust, the majority first tries to install a Rust toolchain using their package manager. And that's only one case of people that I know about who might be stuck with an older Rust toolchain. There might be others. Keeping MSRV low means to cater to the needs of these.

@01mf02
Copy link
Owner Author

01mf02 commented Aug 5, 2024

Essentially, the code I had written relied on the fact that the return type of .into() could be inferred. Type inference is something explicitly permitted as breakage. With the addition of IntoIterator for Box<[T]>, this was no longer the case. This specific instance of breakage was caught months ago, which is why a patch was merged and released in April.

Thanks for your explanation, @jhpratt!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants