influxdb-rust icon indicating copy to clipboard operation
influxdb-rust copied to clipboard

Add time crate support for Timestamp

Open pbzweihander opened this issue 1 year ago • 4 comments

Description

{ describe your changes here }

Checklist

  • [x] Formatted code using cargo fmt --all
  • [x] Linted code using clippy
    • [x] with reqwest feature: cargo clippy --manifest-path influxdb/Cargo.toml --all-targets --no-default-features --features serde,derive,reqwest-client-rustls -- -D warnings
    • [x] with surf feature: cargo clippy --manifest-path influxdb/Cargo.toml --all-targets --no-default-features --features serde,derive,hyper-client -- -D warnings
  • [ ] Updated README.md using cargo doc2readme -p influxdb --expand-macros
  • [x] Reviewed the diff. Did you leave any print statements or unnecessary comments?
  • [x] Any unfinished work that warrants a separate issue captured in an issue with a TODO code comment

pbzweihander avatar Mar 25 '24 01:03 pbzweihander

Thanks, adding time support is something I wanted to see for a long time but never got around to it!

I haven't actually checked where exactly we use chrono, but if it's only for the Timestamp, do you think you could make both chrono and time optional? I feel like most users wouldn't need both at the same time

msrd0 avatar Mar 29 '24 07:03 msrd0

@msrd0 I've updated the PR!

pbzweihander avatar Mar 29 '24 07:03 pbzweihander

The tests need to be adapted to test both chrono and time independently. The tests should enable the features. Ci is currently failing since chrono could not be found in the test.

Empty2k12 avatar Apr 02 '24 11:04 Empty2k12

The tests need to be adapted to test both chrono and time independently.

I'm not sure that's necessary. These two features don't seem to conflict each other.

msrd0 avatar Apr 02 '24 11:04 msrd0