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-41134: Retrieve Data #6

Merged
merged 11 commits into from
Jul 18, 2024
Merged

DOCSP-41134: Retrieve Data #6

merged 11 commits into from
Jul 18, 2024

Conversation

mcmorisi
Copy link
Collaborator

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-41134
Staging - https://preview-mongodbmcmorisi.gatsbyjs.io/kotlin-sync/DOCSP-41134/read/retrieve/

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 Jul 10, 2024

👷 Deploy request for docs-kotlin-sync pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 45f82c5

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 things!

source/read/retrieve.txt Outdated Show resolved Hide resolved
source/read/retrieve.txt Outdated Show resolved Hide resolved
source/read/retrieve.txt Outdated Show resolved Hide resolved
source/read/retrieve.txt Outdated Show resolved Hide resolved
source/read/retrieve.txt Outdated Show resolved Hide resolved
source/read/retrieve.txt Outdated Show resolved Hide resolved
source/read/retrieve.txt Outdated Show resolved Hide resolved
source/read/retrieve.txt Outdated Show resolved Hide resolved
source/read/retrieve.txt Outdated Show resolved Hide resolved
source/read/retrieve.txt Show resolved Hide resolved
@mcmorisi mcmorisi requested a review from rustagir July 10, 2024 21:34
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.

Left some more suggestions!

The ``find()`` method retrieves documents from a collection. This
method takes a **query filter** and returns all matching documents. A query filter is a
document that specifies the criteria that the driver uses to match documents from the
collection.

.. To learn more about query filters, see :ref:`kotlin-sync-specify-query`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: Add a TODO here because it makes these easy to find if they fall through the cracks

// end-find

// start-find-iterate
val resultsToPrint = collection.find(eq("cuisine", "Spanish"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: even though it produces errors in the IDE, can you call all of these variables just results

Also applies to lines 26, 32, 36

Comment on lines 64 to 69
.. literalinclude:: /includes/read/retrieve.kt
:start-after: start-find-iterate
:end-before: end-find-iterate
:language: kotlin
:copyable:
:dedent:
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: convert this to an IO code block and show some sample documents returned

// end-find-all

// start-modified-find
val modifiedResults = collection.find(eq("cuisine", "Spanish")).limit(10).maxTime(10000)
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: reduce horizontal scroll

Suggested change
val modifiedResults = collection.find(eq("cuisine", "Spanish")).limit(10).maxTime(10000)
val modifiedResults = collection
.find(eq("cuisine", "Spanish"))
.limit(10)
.maxTime(10000)

Comment on lines 101 to 102
* - ``collation()``
- | An instance of the ``Collation`` class that sets the collation options.
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: change all descriptions to be the same part of speech

Suggested change
* - ``collation()``
- | An instance of the ``Collation`` class that sets the collation options.
* - ``collation()``
- | Sets the collation options for the query.

Comment on lines 104 to 108
* - ``comment()``
- | A string to attach to the query. This can help you trace and interpret the
operation in the server logs and in profile data. To learn more about query comments,
see :manual:`$comment </reference/operator/query/comment/>` in the MongoDB Server
manual.
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
* - ``comment()``
- | A string to attach to the query. This can help you trace and interpret the
operation in the server logs and in profile data. To learn more about query comments,
see :manual:`$comment </reference/operator/query/comment/>` in the MongoDB Server
manual.
* - ``comment()``
- | Specifies a string to attach to the query. This can help you trace and interpret the
operation in the server logs and in profile data. To learn more about query comments,
see :manual:`$comment </reference/operator/query/comment/>` in the MongoDB Server
manual.

Copy link
Collaborator

Choose a reason for hiding this comment

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

and so on

Comment on lines 144 to 145
For runnable code examples of retrieving documents with the {+driver-short+}, see
:ref:`kotlin-sync-read`.
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
For runnable code examples of retrieving documents with the {+driver-short+}, see
:ref:`kotlin-sync-read`.
To view runnable code examples that retrieve documents by using the {+driver-short+}, see
:ref:`kotlin-sync-read`.

@mcmorisi mcmorisi requested a review from rustagir July 11, 2024 17:14
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.

Great work! thanks for addressing all my feedback in a timely way

free MongoDB Atlas cluster and load the sample datasets, see the
:atlas:`Get Started with Atlas </getting-started>` guide.

This data is modeled with the following Kotlin data class:
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: use language source constant where applicable

Suggested change
This data is modeled with the following Kotlin data class:
The documents in this collection are modeled by the following {+language+} data class:

@mcmorisi mcmorisi requested review from a team and vbabanin and removed request for a team July 11, 2024 20:50
@tom-selander tom-selander requested review from rozza and removed request for vbabanin July 17, 2024 14:55
Copy link
Member

@rozza rozza left a comment

Choose a reason for hiding this comment

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

LGTM!

@mcmorisi mcmorisi merged commit 1db6789 into mongodb:master Jul 18, 2024
1 of 2 checks passed
@mcmorisi mcmorisi deleted the DOCSP-41134 branch July 18, 2024 13:12
mcmorisi added a commit that referenced this pull request Jul 18, 2024
(cherry picked from commit 1db6789)
mcmorisi added a commit that referenced this pull request Jul 18, 2024
(cherry picked from commit 1db6789)
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