georust / gpx

Rust read/write support for GPS Exchange Format (GPX)
https://crates.io/crates/gpx
MIT License
102 stars 46 forks source link

expose `Time` #76

Closed oppiliappan closed 2 years ago

oppiliappan commented 2 years ago

not sure if this is intentionally private, perhaps there is some other way to access time data from waypoints directly as chrono types?

closes #75 if valid

lnicola commented 2 years ago

The field should probably be public as well? Or it could have an into_inner() or impl From<Time> for time::OffsetDateTime, because otherwise you won't be able to do anything with it.

oppiliappan commented 2 years ago

yep, that too, currently i am just using it to compare two times, which is possible with just making it public, but anything else will require being able to convert to OffsetDateTime

oppiliappan commented 2 years ago

@lnicola would you prefer if i create an into_inner or a From<Time> for OffsetDateTime?

lnicola commented 2 years ago

No strong opinion, since both expose the internal representation. I suppose From is slightly more idiomatic. Let's see what the maintainers think (I'm around only incidentally, I've never used this crate).

oppiliappan commented 2 years ago

Likewise, I do like From more.

michaelkirk commented 2 years ago

This LGTM FWIW.

I'd say you are the most active maintainer in this repo @lnicola, so I think it makes sense to merge it if you think it looks good.

lnicola commented 2 years ago

bors r+

lnicola commented 2 years ago

bors r-

bors[bot] commented 2 years ago

Canceled.

lnicola commented 2 years ago

bors r=michaelkirk,lnicola

bors[bot] commented 2 years ago

Build succeeded: