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

Use Duration in a few more places #1038

Merged
merged 1 commit into from
Jul 28, 2023

Conversation

paolobarbolini
Copy link
Contributor

Depends on #1037

@paolobarbolini
Copy link
Contributor Author

Ah right. I forgot the default serializer implementation is useless, and serde_nanos doesn't support Options

@caspervonb
Copy link
Collaborator

caspervonb commented Jul 18, 2023

serde_nanos doesn't support Options

Did add support for Option a while ago, but we've been adding things as we need them so might've missed something, which case is failing?
Could you file an issue or PR against https://github.com/caspervonb/serde_nanos ?

@paolobarbolini
Copy link
Contributor Author

Ah I didn't realize how the API worked. I assumed it was like how time does it by having you go through a different module. I'll update the PR

@Jarema
Copy link
Member

Jarema commented Jul 27, 2023

@paolobarbolini Judging by commits in the PR, I guess some gremlin sneaked while rebasing the branch ;).

@paolobarbolini
Copy link
Contributor Author

Fixed

Copy link
Member

@Jarema Jarema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Jarema Jarema merged commit eb4d492 into nats-io:main Jul 28, 2023
13 checks passed
@Jarema Jarema mentioned this pull request Sep 21, 2023
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.

3 participants