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

Impl (De)Serialize for SystemTime when serde feature is enabled #26

Closed
wants to merge 16 commits into from

Conversation

Ekleog
Copy link
Contributor

@Ekleog Ekleog commented Feb 20, 2024

This is #24 with the last review comments handled.

I did not comment on each replacement of serde's tri! macro with the ? operator.

Kudos to @Fedeparma74 for doing the actual work here, I basically didn't do anything here besides from handling the very few comments that were left and fixing CI :)

Closes #24

@Ekleog
Copy link
Contributor Author

Ekleog commented Feb 20, 2024

@daxpedda This PR is ready for review, I think I handled all that was remaining from #24 :)

Just to note, given that the extra_cmd addition would have required changes in the github required checks anyway, I also took advantage of it to rename the checks into something less prone to change, which will require resetting the required checks this time but hopefully never thereafter.

@Fedeparma74
Copy link
Contributor

Thank you @Ekleog for taking care of this!

@daxpedda
Copy link
Owner

Closing in favor of #28.

@daxpedda daxpedda closed this Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants