-
Notifications
You must be signed in to change notification settings - Fork 56
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-47795] Change batchsize description #392
base: master
Are you sure you want to change the base?
[DOCSP-47795] Change batchsize description #392
Conversation
✅ Deploy Preview for docs-mongodb-shell ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Thanks for these updates, @shuangela! I've left a question and some suggestions for your consideration.
Thanks!
Joe
source/reference/methods.txt
Outdated
limit than 16 MiB, but not a larger one. If you set ``batchSize`` | ||
to a limit that results in batches larger than 16 MiB, this option has no effect. | ||
|
||
A batchSize of 0 means that the cursor will be established, but no documents |
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.
[style suggestion]
Consider wrapping "batchSize" and "0" in backticks here on line 507.
source/reference/methods.txt
Outdated
limit than 16 MiB, but not a larger one. If you set ``batchSize`` | ||
to a limit that results in batches larger than 16 MiB, this option has no effect. | ||
|
||
A batchSize of 0 means that the cursor will be established, but no documents |
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.
[tense suggestion]
Consider updating this to use the present tense. Spitballing:
A ``batchsize`` of ``0`` means that the cursor is established, but no documents
are returned in the first batch.
Thanks for the review, @jmd-mongo! Addressed your feedback and applied it to other pages as well, back to you! |
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.
Thanks, @shuangela! This one LGTM mod a tiny nit.
Thanks,
Joe
source/reference/methods.txt
Outdated
size is ``101`` documents and subsequent batches have a maximum | ||
size of 16 mebibytes (MiB). This option can enforce a smaller | ||
limit than 16 MiB, but not a larger one. If you set ``batchSize`` | ||
to a limit that results in batches larger than 16 MiB, this option has no effect and :method:`~cursor.batchSize()` uses the default batch size. |
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.
[nit]
Would you mind wrapping this line at col 72? Thanks!
@ChrisHarris2012 Thank you for offering to review! |
DESCRIPTION
Updates the reference/methods page to clarify how BatchSize works. Related to the batchsize pr in the docs repo. No new sections were added, but descriptions of BatchSize were changed.
Wording taken from already updated PHP driver docs which was approved by a PHP engineer.
STAGING
https://deploy-preview-392--docs-mongodb-shell.netlify.app/reference/methods/#cursor-methods
JIRA
https://jira.mongodb.org/browse/DOCSP-47795
BUILD LOG
https://app.netlify.com/sites/docs-mongodb-shell/deploys/67d4525e66989c0008faf1e8
Self-Review Checklist
External Review Requirements
What's expected of an external reviewer?