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

DOCSP-41865: datetime serializers #173

Merged
merged 5 commits into from
Sep 24, 2024

Conversation

rustagir
Copy link
Collaborator

@rustagir rustagir commented Sep 19, 2024

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-41865
Staging:

note that the test is failing because of #170

Self-Review Checklist

  • Is this free of any warnings or errors in the RST?
  • Did you run a spell-check?
  • Did you run a grammar-check?
  • Are all the links working?
  • Are the facets and meta keywords accurate?

Copy link

netlify bot commented Sep 19, 2024

Deploy Preview for docs-kotlin ready!

Name Link
🔨 Latest commit b1281e5
🔍 Latest deploy log https://app.netlify.com/sites/docs-kotlin/deploys/66f18bf59540640008745ed8
😎 Deploy Preview https://deploy-preview-173--docs-kotlin.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@mongoKart mongoKart left a comment

Choose a reason for hiding this comment

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

nice job. a few thoughts and questions

Comment on lines 316 to 319
``kotlinx-datetime`` is a {+language+} library that allows
you to work with dates and times. If you want a high level of control
over how your date and time values are serialized, you can add the
dependency to your project's dependency list.
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: maybe a little more straightforward

Suggested change
``kotlinx-datetime`` is a {+language+} library that allows
you to work with dates and times. If you want a high level of control
over how your date and time values are serialized, you can add the
dependency to your project's dependency list.
``kotlinx-datetime`` is a {+language+} library that offers
a high level of control over how your date and time values
are serialized. To use the library, add the ``kotlinx-datetime``
dependency to your project's dependency list.


.. sharedinclude:: dbx/jvm/v5.2-wn-items.rst

.. replacement:: avs-index-link

:ref:`kotlin-search-indexes` in the Indexes guide

- Adds support for serializers from the ``kotlinx-datetime`` library
that let you map to BSON as the expected types instead of as strings.
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: specify type(s)

Suggested change
that let you map to BSON as the expected types instead of as strings.
that let you map ``LocalDate``[?] values to BSON as the expected types instead of as strings.

- Adds support for serializers from the ``kotlinx-datetime`` library
that let you map to BSON as the expected types instead of as strings.
To learn more, see the :ref:`kotlin-datetime-serialization` section of
the Kotlin Serialization guide.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could use source constant here

Suggested change
the Kotlin Serialization guide.
the {+language+} Serialization guide.

Comment on lines 357 to 359
The following ``Appointment`` data class has the ``@Serializable``
annotation and the ``@Contextual`` annotation on the ``date`` field
to use the ``kotlinx-datetime`` serializer for ``LocalDate`` values.
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: i had a little trouble parsing this all at first. i tried a few versions and ended up with a list, but feel free to use/discard parts as you see fit. it also sort of bleeds into the following example about the time being stored as a string.

Suggested change
The following ``Appointment`` data class has the ``@Serializable``
annotation and the ``@Contextual`` annotation on the ``date`` field
to use the ``kotlinx-datetime`` serializer for ``LocalDate`` values.
In the following code example, the driver serializes all fields in
the ``Appointment`` data class in the following ways:
- ``name``: The driver serializes the value as a string.
- ``date``: The driver uses the ``kotlinx-datetime`` serializer because
the field includes the ``@Contextual`` annotation. The value is serialized
as a BSON date.
- ``time``: The driver serializes the value as a string.

The following ``Appointment`` data class has the ``@Serializable``
annotation and the ``@Contextual`` annotation on the ``date`` field
to use the ``kotlinx-datetime`` serializer for ``LocalDate`` values.
Values of the ``time`` field are stored as strings, because the driver
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q: Does this code example show that the time field is stored as a string?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, this is the default behavior of the driver. The result is shown later in the section

Copy link
Collaborator

@mongoKart mongoKart left a comment

Choose a reason for hiding this comment

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

LGTM w/ 1 suggestion

Comment on lines 363 to 364
because the field has the ``@Contextual`` annotation. So,
``LocalDate`` values are serialized as BSON dates.
Copy link
Collaborator

Choose a reason for hiding this comment

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

S:

Suggested change
because the field has the ``@Contextual`` annotation. So,
``LocalDate`` values are serialized as BSON dates.
because the field has the ``@Contextual`` annotation.
``LocalDate`` values are serialized as BSON dates.

Copy link
Member

@rozza rozza left a comment

Choose a reason for hiding this comment

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

LGTM!

@rustagir rustagir merged commit e0b1e01 into mongodb:master Sep 24, 2024
7 of 8 checks passed
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