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-26552 - polymorphism #143

Merged
merged 10 commits into from
Nov 30, 2023

Conversation

mongoKart
Copy link
Collaborator

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-26552
Staging - https://preview-mongodbmongokart.gatsbyjs.io/csharp/docsp-26552-polymorphism/fundamentals/data-formats/polymorphism/

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
Collaborator Author

@mongoKart mongoKart Nov 30, 2023

Choose a reason for hiding this comment

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

Carefully reviewed all Vale errors in this file (ignored other files for now) and didn't see a way to avoid them that's clear, concise, and grammatical. Style Guide rationale for avoiding subjunctive mood (avoiding uncertainty) also doesn't apply here. Open to suggestions, though!

Copy link
Collaborator

@rachel-mack rachel-mack left a comment

Choose a reason for hiding this comment

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

A few minor things:

Comment on lines +101 to +103
If you're creating a class map manually, call the
``BsonClassMap.RegisterClassMap<T>()`` method for every class in the hierarchy, as shown
in the following example:
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could include a link to the Class Mapping page here.

Comment on lines 134 to 137
Suppose you created an instance of the example ``Animal`` class and each of its
subclasses. If you serialized these objects to a single collection, the {+driver-short+}
would apply the ``ScalarDiscriminatorConvention`` and the corresponding
BSON documents would appear as follows:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree that what you've written here is fine, but to get around Vale you could change it to present tense:

Suggested change
Suppose you created an instance of the example ``Animal`` class and each of its
subclasses. If you serialized these objects to a single collection, the {+driver-short+}
would apply the ``ScalarDiscriminatorConvention`` and the corresponding
BSON documents would appear as follows:
Suppose you create an instance of the example ``Animal`` class and each of its
subclasses. If you serialize these objects to a single collection, then the
{+driver-short+} applies the ``ScalarDiscriminatorConvention`` and the corresponding
BSON documents appear as follows:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought about this formation a lot last night and whether it was strictly grammatical, or whether it should be "If you were to serialize..." and so on. But this is definitely readable and simplifies the sentence so I'm down with it

{
}

If you're creating a class map manually, call the ``SetIsRootClass()`` method and pass
Copy link
Collaborator

Choose a reason for hiding this comment

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

You've used this consistently, I just want to confirm that the method is SetIsRootClass(), rather than SetAsRootClass().

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep. Thanks for checking

Comment on lines 198 to 202
Suppose you labeled the example ``Animal`` class as the root of the inheritance hierarchy,
and then created an instance of the ``Animal`` class and each of its
subclasses. If you serialized these objects to a single collection, the {+driver-short+}
would apply the ``HierarchicalDiscriminatorConvention`` and the corresponding
BSON documents would appear as follows:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use present tense again to avoid Vale:

Suggested change
Suppose you labeled the example ``Animal`` class as the root of the inheritance hierarchy,
and then created an instance of the ``Animal`` class and each of its
subclasses. If you serialized these objects to a single collection, the {+driver-short+}
would apply the ``HierarchicalDiscriminatorConvention`` and the corresponding
BSON documents would appear as follows:
Suppose you labele the example ``Animal`` class as the root of the inheritance hierarchy,
and then create an instance of the ``Animal`` class and each of its
subclasses. If you serialize these objects to a single collection, the {+driver-short+}
applies the ``HierarchicalDiscriminatorConvention`` and the corresponding
BSON documents appear as follows:


If you're working with data that doesn't follow the conventions used by the
{+driver-short+}--for example, data inserted into MongoDB by another driver or object
mapper--you might need to use a different value for your discriminator field to
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know how to rewrite this one - I prefer "might" to "may" and I don't actually see where in the style guide this is written as a rule.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, "must" doesn't have a direct subjunctive equivalent. leaving as is

@mongoKart mongoKart merged commit 139fe47 into mongodb:master Nov 30, 2023
1 of 2 checks passed
@mongoKart mongoKart deleted the docsp-26552-polymorphism branch November 30, 2023 21:53
mongoKart added a commit that referenced this pull request Nov 30, 2023
(cherry picked from commit 139fe47)
mongoKart added a commit that referenced this pull request Nov 30, 2023
(cherry picked from commit 139fe47)
mongoKart added a commit that referenced this pull request Nov 30, 2023
(cherry picked from commit 139fe47)
mongoKart added a commit that referenced this pull request Nov 30, 2023
(cherry picked from commit 139fe47)
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.

2 participants