Skip to content

Conversation

rustagir
Copy link
Contributor

@rustagir rustagir commented Oct 30, 2024

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-44849
Staging - https://deploy-preview-59--docs-mongoid.netlify.app/interact-data/modify-results/

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 Oct 30, 2024

Deploy Preview for docs-mongoid ready!

Name Link
🔨 Latest commit 1616f65
🔍 Latest deploy log https://app.netlify.com/sites/docs-mongoid/deploys/6724e471559af300089f3ce3
😎 Deploy Preview https://deploy-preview-59--docs-mongoid.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@mongoKart mongoKart left a comment

Choose a reason for hiding this comment

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

nothing critical, but i'm curious about enough stuff that i'd like to see it again!

Comment on lines 99 to 101
You can pass fields of referenced associations to the ``only()`` method,
but the projection is ignored when loading the embedded objects. {+odm+}
loads all fields of the referenced associations.
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: might be my lack of ruby experience, but i'm still not sure i understand this paragraph. will ruby people get it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It means that even if you project only one field of an embedded document, the whole thing will load when you access it from the result. I can reword

Comment on lines 207 to 209
If you chain sort specifications, the first call defines the most
significant criteria and the newest call defines the least significant
one.
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: does most/least significant mean the order in which ruby will try to sort the fields by (sort the most significant, then second most, etc.)? I just haven't heard these terms before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ill reword

.. note:: Sorting in Scopes

If you define a scope on your model that includes a sort specification,
the scope sort takes precedence over the sort specified in a query, as the
Copy link
Contributor

Choose a reason for hiding this comment

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

I: as

Suggested change
the scope sort takes precedence over the sort specified in a query, as the
the scope sort takes precedence over the sort specified in a query, because the

----------------

{+odm+} provides the ``limit()``, ``skip()``, and ``batch_size()``
pagination operators that you can use on ``Criteria`` objects. The
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: operators vs methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are interchangeable

Comment on lines 231 to 232
You can limit the number of results that {+odm+} returns by using the
``limit()`` method.
Copy link
Contributor

Choose a reason for hiding this comment

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

S: slight ambiguity between you using the limit() method and mongoid using it

Suggested change
You can limit the number of results that {+odm+} returns by using the
``limit()`` method.
You can use the ``limit()`` method to limit the number of results that {+odm+} returns.

Comment on lines 246 to 247
method, or its alias ``offset()``. If you chain a ``limit()`` call, it
is applied after documents are skipped.
Copy link
Contributor

Choose a reason for hiding this comment

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

S: might be useful to have an example or admonition for this, actually! probably a common question.

Suggested change
method, or its alias ``offset()``. If you chain a ``limit()`` call, it
is applied after documents are skipped.
method, or its alias ``offset()``. If you chain a ``limit()`` call
to the ``skip()`` method, the limit
is applied after documents are skipped.

@rustagir rustagir requested a review from mongoKart November 1, 2024 14:23
Copy link
Contributor

@mongoKart mongoKart left a comment

Choose a reason for hiding this comment

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

LGTM!

@rustagir rustagir merged commit 1a5bac2 into mongodb:standardized Nov 1, 2024
5 checks passed
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