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-44005 - DateTimeOffset #245

Merged
merged 5 commits into from
Oct 7, 2024

Conversation

mongoKart
Copy link
Collaborator

@mongoKart mongoKart commented Oct 1, 2024

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-44005

Staging Links

  • upgrade/v3
  • 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 Oct 1, 2024

    Deploy Preview for mongodb-docs-csharp ready!

    Name Link
    🔨 Latest commit e4f6b38
    🔍 Latest deploy log https://app.netlify.com/sites/mongodb-docs-csharp/deploys/6703fa121c5358000860071c
    😎 Deploy Preview https://deploy-preview-245--mongodb-docs-csharp.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

    @stephmarie17 stephmarie17 left a comment

    Choose a reason for hiding this comment

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

    LGTM ✅

    @mongoKart mongoKart requested review from a team and sanych-sun and removed request for a team October 2, 2024 13:10
    API reference page on MSDN.

    - Serializes ``DateTimeOffset`` values as BSON documents, and the
    ``DateTime`` values in these documents as milliseconds since the Unix epoch.
    Copy link

    Choose a reason for hiding this comment

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

    I would say that either we just say that now the default serialization is document, or if we want to go into details we should specify all the document fields:

    • DateTime (milliseconds since Unix Epoch)
    • Ticks (the ticks of the value)
    • Offset (the total minutes of the value)
      I don't have a strong preference for either of those, and if users already know what's inside the document (that has not changed this version), then we can just say we changed the default

    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    Should we consider this a breaking change?

    Copy link

    Choose a reason for hiding this comment

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

    Yes, this is a breaking change

    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    Moved to the v3 Upgrade page.

    @mongoKart mongoKart removed the request for review from sanych-sun October 3, 2024 17:22
    @mongoKart mongoKart requested a review from papafe October 3, 2024 17:23
    driver versions serialized these values as BSON arrays.

    For more information about this release, see the :github:`v3.0 release notes
    </mongodb/mongo-csharp-driver/releases/tag/v3.0.0>`.
    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    Link broken until APIs are published

    Copy link

    @papafe papafe left a comment

    Choose a reason for hiding this comment

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

    LGTM

    @mongoKart mongoKart merged commit 543e9c2 into mongodb:v3.0 Oct 7, 2024
    5 of 6 checks passed
    @mongoKart mongoKart deleted the docsp-44005-datetimeoffset branch October 7, 2024 15:11
    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