Skip to content

Conversation

rustagir
Copy link
Contributor

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-22844
Staging - https://preview-mongodbrustagir.gatsbyjs.io/java/DOCSP-33844-pojo-examples/fundamentals/data-formats/document-data-format-pojo/

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
Contributor

@norareidy norareidy left a comment

Choose a reason for hiding this comment

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

Looks really good! A few suggestions / questions from me

Comment on lines 72 to 75
- ``PojoCodecProvider``, a codec provider that includes
:ref:`Codecs <fundamentals-codecs>` that define how to
encode and decode the data between the POJO format and BSON. The
provider also specifies which POJO classes or packages that the codecs
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: is Codec uppercase or lowercase? Both are used in this description

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed all non-monospace to lowercase

provider also specifies which POJO classes or packages that the codecs
apply to.
- ``CodecRegistry`` instance that contains the codecs and other related information.
- ``MongoDatabase`` or ``MongoCollection`` instance configured to use the
Copy link
Contributor

Choose a reason for hiding this comment

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

S: to match the wording in the bullet below

Suggested change
- ``MongoDatabase`` or ``MongoCollection`` instance configured to use the
- ``MongoDatabase`` or ``MongoCollection`` instance that is configured to use the

Comment on lines 83 to 84
The following steps perform each of the configuration requirements to
use the sample POJO defined in the preceding section:
Copy link
Contributor

Choose a reason for hiding this comment

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

S: the wording here kinda implies that the step itself is the subject taking some action; you could reword so that the reader is the subject and the one taking action

Suggested change
The following steps perform each of the configuration requirements to
use the sample POJO defined in the preceding section:
Perform the following steps to meet the configuration requirements defined in the preceding section:

Comment on lines 128 to 129
Codecs in the ``CodecRegistry``. You can configure either a database
or collection to specify the Codecs.
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: same codec capitalization question as above


- Instantiate a ``PojoCodecProvider`` which contains codecs which define how to
- Instantiate a ``PojoCodecProvider`` which contains codecs that define how to
encode/decode data between BSON and the POJO fields.
Copy link
Contributor

Choose a reason for hiding this comment

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

S: to remove the slash

Suggested change
encode/decode data between BSON and the POJO fields.
encode and decode data between BSON and the POJO fields.

Comment on lines 72 to 73
- ``PojoCodecProvider``, a codec provider that includes
:ref:`Codecs <fundamentals-codecs>` that define how to
Copy link
Contributor

Choose a reason for hiding this comment

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

S: a little repetitive

Suggested change
- ``PojoCodecProvider``, a codec provider that includes
:ref:`Codecs <fundamentals-codecs>` that define how to
- ``PojoCodecProvider``, which provides
:ref:`Codecs <fundamentals-codecs>` that define how to

@rustagir rustagir requested a review from norareidy January 25, 2024 16:26
Copy link
Contributor

@norareidy norareidy left a comment

Choose a reason for hiding this comment

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

LGTM + suggestion!

@@ -238,12 +209,13 @@ see the following API documentation:
Summary
-------

In this guide, we explained how to convert data between BSON and POJO fields
by performing the following:
In this guide, we explain how to convert data between BSON and POJO fields
Copy link
Contributor

Choose a reason for hiding this comment

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

S: since the style guide suggests limiting "we", you could reword to something like:

Suggested change
In this guide, we explain how to convert data between BSON and POJO fields
This guide describes how to convert data between BSON and POJO fields

@rustagir rustagir merged commit 992dda6 into mongodb:master Jan 25, 2024
rustagir added a commit that referenced this pull request Jan 25, 2024
* DOCSP-22844: expand pojo example+revisions

* full first draft

* small fixes

* add taxonomy

* vale fix

* NR PR fixes 1

* NR suggestion

(cherry picked from commit 992dda6)
rustagir added a commit that referenced this pull request Jan 25, 2024
* DOCSP-22844: expand pojo example+revisions

* full first draft

* small fixes

* add taxonomy

* vale fix

* NR PR fixes 1

* NR suggestion

(cherry picked from commit 992dda6)
rustagir added a commit that referenced this pull request Jan 25, 2024
* DOCSP-22844: expand pojo example+revisions

* full first draft

* small fixes

* add taxonomy

* vale fix

* NR PR fixes 1

* NR suggestion

(cherry picked from commit 992dda6)
rustagir added a commit that referenced this pull request Jan 25, 2024
* DOCSP-22844: expand pojo example+revisions

* full first draft

* small fixes

* add taxonomy

* vale fix

* NR PR fixes 1

* NR suggestion

(cherry picked from commit 992dda6)
rustagir added a commit that referenced this pull request Jan 25, 2024
* DOCSP-22844: expand pojo example+revisions

* full first draft

* small fixes

* add taxonomy

* vale fix

* NR PR fixes 1

* NR suggestion

(cherry picked from commit 992dda6)
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