Codecov Report

Merging #581 (8d8e609) into main (e83d01f) will decrease coverage by 0.3%.
The diff coverage is 76.7%.

@@           Coverage Diff           @@
##            main    #581     +/-   ##
=======================================
- Coverage   95.7%   95.5%   -0.3%     
=======================================
  Files         79      79             
  Lines       8795    8906    +111     
=======================================
+ Hits        8421    8504     +83     
- Misses       374     402     +28     

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Is there any interest in this method for handling serde in a nice way? I'm happy to progress this to handle all serde implementations, but if it's not the path to go down I'd rather not spend the time.

Looking strictly at the public API (via generated rustdoc), I have some questions and concerns.

• PrimitiveDateTime should not be supported for timestamps, as they have no concept of time zones. It would probably be best to make the underlying type a generic, which would allow
• Timestamps with milliseconds has been previously rejected as unnecessary. This is particularly so now that there is time::serde::format_description! and a [unix_timestamp] component. I'm open to reconsidering it as a serde format, but not as a method.
• Is there any situation where a type would implement one trait but not the other?
• Let's keep the new unit type and the traits inaccessible. They'll have to be public for the purposes of the API, but that doesn't mean a user needs the ability to access the items. This would avoid changing the public API in any manner.

1. Fair point, I'll take it out.
2. I think it's worth re-assessing this since the way this approach works doesn't then blow out the module count into time::serde::timestamp::{self,milliseconds}::{option,vec,...}. It's also common enough for web interactions since JS uses milliseconds for time. My initial need for this was that I needed a Vec<OffsetDateTime> from a web form and it was quite involved to make the custom milliseconds -> OffsetDateTime function, then pull in serde-with, wrap it all up and add it. But if the consensus is to leave it out I'm happy to not block the PR on that specific point.
3. Yep, it's included in the PR: [T] can implement AsWellKnown but not FromWellKnown.
4. I like that idea, if you could point me to an example of how to do this, I'd much appreciate it!

Thanks for the review!

For how to limit the ability to access something:

mod private {
    pub struct Timestamp;
    pub trait AsWellKnown {}
}

use private::Timestamp;
use private::AsWellKnown;

So long as there's never a pub use, the items remain inaccessible from other crates despite being public. This is because any use would have to include the private module.

Would you mind providing an update?

Closing as inactive.