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-37612: v6.5.0 release #881

Merged
merged 5 commits into from
Mar 12, 2024

Conversation

ccho-mongodb
Copy link
Contributor

@ccho-mongodb ccho-mongodb commented Mar 11, 2024

Pull Request Info

PR Reviewing Guidelines

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

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

@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.

A few small things, but I'll take another look if you want!

Comment on lines 67 to 68
- Moved ``ObjectId`` generation to the ``pkFactory`` class for bulk write
operations.
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: match tense

Suggested change
- Moved ``ObjectId`` generation to the ``pkFactory`` class for bulk write
operations.
- Moves ``ObjectId`` generation to the ``pkFactory`` class for bulk write
operations.

Comment on lines 72 to 74
If you previously specified the ``pkFactory`` to handle bulk writes,
the ``_id`` fields of the documents inserted by using bulk writes may
be inconsistent with the behavior in this version.
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
If you previously specified the ``pkFactory`` to handle bulk writes,
the ``_id`` fields of the documents inserted by using bulk writes may
be inconsistent with the behavior in this version.
If you previously specified that a ``pkFactory`` instance handles bulk writes,
the ``_id`` fields of the documents inserted by using bulk writes may
be inconsistent with the behavior in this version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call! Will update to clarify an instance.

- Moved ``ObjectId`` generation to the ``pkFactory`` class for bulk write
operations.

.. warning::
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q: should this warning be indented to be under the preceding bullet point?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure you can put an admonition in a list, but I'll try it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, that didn't work (unexpected indentation error)

Comment on lines 76 to 77
- Fixes the read preference sent with read operations when directly connected
to a secondary in the replica set to ``primaryPreferred``.
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
- Fixes the read preference sent with read operations when directly connected
to a secondary in the replica set to ``primaryPreferred``.
- Sets the read preference that is sent with read operations to ``primaryPreferred`` when
you are directly connected to a secondary in the replica set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the rewording. I'll use parts of this suggestion, but will continue to use the verb "fixes" to make sure it's understood that way. From the release notes:

Fixed applying read preference to commands depending on topology

Comment on lines +93 to +96
To learn more about this release, see the
`v6.5.0 Release Highlights
<https://github.com/mongodb/node-mongodb-native/releases/tag/v6.5.0>`__ on
GitHub.
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 out of scope, but I noticed the v6.4 section is missing this link

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I'll add it.

Copy link

@baileympearson baileympearson left a comment

Choose a reason for hiding this comment

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

one wording change, otherwise looks good

Comment on lines 67 to 68
- Moves ``ObjectId`` generation to the ``pkFactory`` class for bulk write
operations.

Choose a reason for hiding this comment

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

Suggested change
- Moves ``ObjectId`` generation to the ``pkFactory`` class for bulk write
operations.
- Bulk write operations generate document ids using the ``pkFactory``

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll change "ObjectID generation" to "generate document ids".
To maintain the style, the bullet point needs to start with a present tense verb. I'll change it to the following (feel free to suggest something else):

"Updates bulk write operations to use the pkFactory class for document id generation."

Choose a reason for hiding this comment

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

yup, that works too. thanks!

@ccho-mongodb ccho-mongodb merged commit 241e796 into mongodb:master Mar 12, 2024
2 checks passed
@ccho-mongodb ccho-mongodb deleted the DOCSP-37612-v6.5.0 branch March 12, 2024 15:48
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