-
Notifications
You must be signed in to change notification settings - Fork 10
DOCSP-41133: Specify a Query #11
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
Conversation
👷 Deploy request for docs-kotlin-sync pending review.Visit the deploys page to approve it
|
To learn more about retrieving documents with the {+driver-short+}, see | ||
:ref:`kotlin-sync-retrieve`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note for reviewers: The retrieve page here is currently also in review. This PR should be merged in after the retrieve page is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on this! LGTM w/ a question and suggestion
source/read/specify-a-query.txt
Outdated
|
||
- ``$text``, which performs a text search on the documents | ||
- ``$regex``, which returns documents that match a specified regular expression | ||
- ``$mod``, which performs a modulo operation on the value of a field and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
S: I think it could be helpful to link to modulo here for those who don't know what it is
- ``$mod``, which performs a modulo operation on the value of a field and | |
- ``$mod``, which performs a :wikipedia:`modulo </Modulo>` operation on the value of a field and |
source/read/specify-a-query.txt
Outdated
The following example specifies a query filter as a parameter to the ``find()`` | ||
method. The code returns all documents with a ``color`` field value of ``"yellow"``: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our guidance isn't 100% clear on what "introducing" means, but the style guide on colons says: "If another sentence intervenes between the introduction and the thing being introduced, use a period instead of a colon." I think that might apply to these examples? Since we say "The following example..." in the first sentence, but then have a clarifying sentence in between.
I can see a case where both sentences are introduction sentences though, so will differ to your judgement on whether or not we change these.
Applies to most sections
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with your interpretation of the style guide here. Can change these to periods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
(cherry picked from commit ffd5d1f)
(cherry picked from commit ffd5d1f)
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-41133
Staging - https://preview-mongodbmcmorisi.gatsbyjs.io/kotlin-sync/DOCSP-41133-specify-query/read/specify-a-query/
Self-Review Checklist