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-43525 - Decimal128 #240

Merged
merged 5 commits into from
Oct 1, 2024
Merged

Conversation

mongoKart
Copy link
Collaborator

@mongoKart mongoKart commented Sep 26, 2024

Pull Request Info

PR Reviewing Guidelines

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

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 Sep 26, 2024

    Deploy Preview for mongodb-docs-csharp ready!

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

    @rustagir rustagir left a comment

    Choose a reason for hiding this comment

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

    few small things

    @@ -102,6 +102,15 @@ Version 3.0 Potential Breaking Change
    - TLS 1.0 and 1.1 are no longer supported. You must use TLS 1.2 or higher. To learn
    more about configuring TLS/SSL in the {+driver-short+}, see :ref:`<csharp-tls>`.

    - In previous versions of the driver, if a field had the C# data type ``Decimal128`` or
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    Suggested change
    - In previous versions of the driver, if a field had the C# data type ``Decimal128`` or
    - In previous versions of the driver, if a field had a value of {+language+} data type ``Decimal128`` or

    Comment on lines 105 to 108
    - In previous versions of the driver, if a field had the C# data type ``Decimal128`` or
    ``decimal``, the driver serialized the value as a BSON ``string`` by default. In
    version 3.0, the driver defaults to serializing these values as BSON ``Decimal128``
    values instead.
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    S: flip these sentence around eg

    Serializes {+language+} Decimal128 values as BSON Decimal128 values. In prev versions...

    Comment on lines 109 to 110
    To serialize a a ``decimal`` or ``Decimal128`` value as a string,
    apply the ``[BsonRepresentation(BsonType.String)]`` attribute to the field.
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    Suggested change
    To serialize a a ``decimal`` or ``Decimal128`` value as a string,
    apply the ``[BsonRepresentation(BsonType.String)]`` attribute to the field.
    To serialize a ``decimal`` or ``Decimal128`` value as a string,
    apply the ``[BsonRepresentation(BsonType.String)]`` attribute to the field.

    Comment on lines 109 to 110
    To serialize a a ``decimal`` or ``Decimal128`` value as a string,
    apply the ``[BsonRepresentation(BsonType.String)]`` attribute to the field.
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    S: Instead of boradly linking to the serialization guide, consider linking to the bsonrepresentation attribute content

    Copy link
    Collaborator

    @rustagir rustagir left a comment

    Choose a reason for hiding this comment

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

    lgtm

    source/upgrade.txt Outdated Show resolved Hide resolved
    @mongoKart mongoKart requested review from a team and sanych-sun and removed request for a team September 27, 2024 16:25
    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 removed the request for review from sanych-sun October 1, 2024 13:42
    @mongoKart mongoKart merged commit 6385e66 into mongodb:v3.0 Oct 1, 2024
    5 of 6 checks passed
    @mongoKart mongoKart deleted the docsp-43525-decimal128 branch October 1, 2024 13:42
    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