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

RUST-1987 Add serde helper module for Option<DateTime> #482

Merged

Conversation

lazureykis
Copy link
Contributor

No description provided.

@lazureykis lazureykis changed the title Support serializing Option<chrono::DateTime<_>> as Option<bson::DateTime> Support serialization of Option<chrono::DateTime> Jul 6, 2024
@isabelatkinson
Copy link
Contributor

Hi @lazureykis, thanks for opening this PR! Can you please add some tests for this functionality to this file?

@lazureykis
Copy link
Contributor Author

Hi @lazureykis, thanks for opening this PR! Can you please add some tests for this functionality to this file?

Added.

Copy link
Contributor

@isabelatkinson isabelatkinson left a comment

Choose a reason for hiding this comment

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

Looks like there are some formatting errors. You can run the check-rustfmt.sh script in the .evergreen directory to determine where the errors are coming from and then use rustfmt +nightly --unstable-features <filename> to fix them.

src/serde_helpers.rs Outdated Show resolved Hide resolved
src/serde_helpers.rs Outdated Show resolved Hide resolved
lazureykis and others added 3 commits July 11, 2024 13:29
@lazureykis
Copy link
Contributor Author

Looks like there are some formatting errors. You can run the check-rustfmt.sh script in the .evergreen directory to determine where the errors are coming from and then use rustfmt +nightly --unstable-features <filename> to fix them.

Thanks. Should be fixed now.

Copy link
Contributor

@isabelatkinson isabelatkinson left a comment

Choose a reason for hiding this comment

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

looks great - going to tag in the other team member for approval and then we can merge this in! thanks for the contribution!

@isabelatkinson isabelatkinson requested a review from abr-egn July 12, 2024 21:43
Copy link
Contributor

@abr-egn abr-egn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for contributing!

@isabelatkinson isabelatkinson changed the title Support serialization of Option<chrono::DateTime> RUST-1987 Add serde helper module for Option<DateTime> Jul 15, 2024
@isabelatkinson isabelatkinson merged commit b554142 into mongodb:main Jul 15, 2024
11 checks passed
@lazureykis lazureykis deleted the support-optional-chrono-datetime branch July 16, 2024 15:59
@lazureykis lazureykis restored the support-optional-chrono-datetime branch July 16, 2024 16:03
@lazureykis lazureykis deleted the support-optional-chrono-datetime branch July 16, 2024 16:03
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