Allow to user chose chrono vs time

[a1ien]

• [x] I agree to follow the project's code of conduct.

• [x] I added an entry to CHANGELOG.md if knowledge of this change could be valuable to users.

[lnicola]

With features being additive in theory, I'm not too fond of the idea of a Time type that changes depending on what is enabled. But switching from chrono to time seems like a good idea, since the former has been less well maintained lately.

I'd also slightly prefer using the formatting or parsing functions in the tests to avoid pulling in a proc macro.

---

Let's see what the other reviewers think.

[a1ien]

I add feature because if we switch from chrono to time we must bump version to 0.9. This is because we have chrono in public interface. So if you ok with switch completely to time I am happy with that.

[a1ien]

Rewrite test without macro

[lnicola]

r? @urschrei

[urschrei]

One thing to note re: breaking changes is that gpx has no public dependents. I know that doesn't mean it has no dependents, but as a practical consideration, it's likely to affect a small number of users. Sometimes a version bump is the correct thing to do, especially in service of longer-term stability / freedom from security advisories.

[a1ien]

So we can bump to 0.9 and remove chrono?

[michaelkirk]

Disclaimer: I don't actively use this crate, so this is non-blocking feedback.

It's kind of unfortunate to see so many #cfg checks scattered throughout the code - is there a way we can centralize them? At least the type alias should be easy to centralize.

Though alternatively, would it be better to use our own internal Time type and offer Into/From impls for the time and chrono crates?

[urschrei]

So we can bump to 0.9 and remove chrono?

We could; I'm not against it, but as @michaelkirk also noted, I'm not a direct user of the crate. We just maintain it as best we can. Please note that there are some strings attached: if you change and bump, please help us to field questions / do some maintenance if it's required 🙂

Though alternatively, would it be better to use our own internal Time type and offer Into/From impls for the time and chrono crates

If this isn't a huge headache and avoids a breaking change, it's the better option.

[michaelkirk]

I'm also not against removing chrono support if people think it's reasonable.

Defining an internal type would still be a breaking change here, but might (might) avoid a breaking change in the future when people decide that now the time crate is old-and-busted and we should all be using 2025's latest time keeping crate: "swatch"

I leave it to whoever is doing the work to decide. =)

[lnicola]

We could use time for our opaque internal time to avoid reinventing the wheel. But a breaking change every couple of years or so doesn't seem so bad either.

[a1ien]

Rework to internal Time type with From/Into conversion. If we decide to use that approach I can squash commit

[michaelkirk]

bors try

[bors[bot]]

try

Build failed:

• ci result

[a1ien]

Fix optional time crate

[michaelkirk]

I'm seeing test failures:

$ cargo test --all-features
   Compiling serde_derive v1.0.133                                                     
   Compiling serde v1.0.133          
   Compiling geo-types v0.7.2              
   Compiling time v0.3.5                                                               
   Compiling geo v0.18.0                                                                                                                                                      
   Compiling gpx v0.8.5 (/Users/mkirk/src/georust/gpx)
error[E0277]: the trait bound `parser::time::Time: Serialize` is not satisfied                                                                                                
    --> src/types.rs:83:5                                                                                                                                                     
     |                                                                                                                                                                        
83   |     /// The creation date of the file.                                                                                                                                 
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Serialize` is not implemented for `parser::time::Time`
     |                

Can you verify that you can run the tests?

[a1ien]

Forgot to implement serde derives for time. Fix.

[michaelkirk]

bors r+

[bors[bot]]

Merge conflict.

[a1ien]

Fix merge conflict and update changelog

[michaelkirk]

bors retry

[bors[bot]]

Build failed:

• ci result

[michaelkirk]

Looks like the time crate requires a newer version of rust. I'm ok with updating the minimum supported rust version, but it's nice to know what it is.

Do you know what the minimum supported rust version is after this PR?

[a1ien]

For current version time 0.3.5 MSRV is 1.5.1 for new unreleased it's 1.53

[michaelkirk]

bors retry

[bors[bot]]

Build succeeded:

• ci result