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

Move the datetime::serde timestamp visitors into the respective modules #669

Merged
merged 1 commit into from
Sep 8, 2022
Merged

Move the datetime::serde timestamp visitors into the respective modules #669

merged 1 commit into from
Sep 8, 2022

Conversation

nickelc
Copy link
Contributor

@nickelc nickelc commented Apr 5, 2022

The internal structs can be moved into their respective modules because pub(super) is now supported by the MSRV.

This is technically a breaking change but these structs were hidden from the docs.

…ules

The internal structs can be moved into their respective modules
because `pub(super)` is now supported by the MSRV.
@djc
Copy link
Member

djc commented Apr 6, 2022

This looks like a good change but I'm a little worried about Hyrum's law (and it doesn't seem that urgent/important), so I'd like to postpone this until at least after the 0.4.20 release and maybe to 0.5.

Any thoughts on that?

@nickelc
Copy link
Contributor Author

nickelc commented Apr 6, 2022

so I'd like to postpone this until at least after the 0.4.20 release and maybe to 0.5.

sure, I'm fine with it.
The move is just to make the datetime serde modules more comparable.

@esheppa esheppa added the API-incompatible Tracking changes that need incompatible API revisions label Jul 3, 2022
@esheppa esheppa added this to the 0.5 milestone Jul 3, 2022
@esheppa
Copy link
Collaborator

esheppa commented Sep 8, 2022

This looks good to me - are you happy to have this merged @djc?

@djc djc merged commit 5305023 into chronotope:main Sep 8, 2022
@djc
Copy link
Member

djc commented Sep 8, 2022

Yes! Thanks.

@nickelc nickelc deleted the move-visitors branch September 8, 2022 14:16
@pitdicker pitdicker mentioned this pull request May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API-incompatible Tracking changes that need incompatible API revisions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants