-
Notifications
You must be signed in to change notification settings - Fork 53
DOCSP-51959 Add a warning about tryNext bug to all change stream manual iteration examples #1189
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-51959 Add a warning about tryNext bug to all change stream manual iteration examples #1189
Conversation
✅ Deploy Preview for docs-node ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🔄 Deploy Preview for docs-node processing
|
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.
One suggestion:
source/includes/try-next-warning.rst
Outdated
The `tryNext() <{+api+}/classes/ChangeStream.html#tryNext>`_ method does not | ||
automatically update the change stream's `resumeToken. | ||
<{+api+}/classes/ChangeStream.html#resumeToken}>`_ If you require an updated | ||
``resumeToken``, use the ``next()`` method. |
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.
The `tryNext() <{+api+}/classes/ChangeStream.html#tryNext>`_ method does not | |
automatically update the change stream's `resumeToken. | |
<{+api+}/classes/ChangeStream.html#resumeToken}>`_ If you require an updated | |
``resumeToken``, use the ``next()`` method. | |
The `tryNext() <{+api+}/classes/ChangeStream.html#tryNext>`_ method does not | |
automatically update the change stream's `resumeToken. | |
<{+api+}/classes/ChangeStream.html#resumeToken>`_ If you require an updated | |
``resumeToken``, use the ``next()`` method. |
@@ -88,6 +88,7 @@ read events from the change stream: | |||
- ``next()`` to request the next document in the stream | |||
- ``close()`` to close the ChangeStream | |||
|
|||
.. include:: /includes/try-next-warning.rst |
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.
There's a warning below the tabs section on line 117/118 (not this one, but I can't comment directly on it), that I would move into the other tabs, because:
- I don't think it's only relevant to the Idiomatic Iteration and Event tabs (ask tech review to confirm)
- It looks a little odd to have two warnings appear in a row
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 believe Iterator
mode applies to idomatic and manual iteration, but I will double check as I agree stacking callouts is odd.
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.
This is relevant to all tabs because it's a warning not to mix the modes defined in each tab, basically.
What do y'all think of moving this above the section altogether?
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 can do that, under where it says "read events from change streams by iterating over them or listening for events." The warning can emphasize the or.
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.
Yeah, I think that works nicely. Thanks
…al iteration examples (mongodb#1189) * DOCSP-51959 tryNext resumeToken error warning * fix link * bracket removal * move warning (cherry picked from commit 7baddf7)
…al iteration examples (mongodb#1189) * DOCSP-51959 tryNext resumeToken error warning * fix link * bracket removal * move warning (cherry picked from commit 7baddf7)
💔 Some backports could not be createdNote: Successful backport PRs will be merged automatically after passing CI. Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
…al iteration examples (mongodb#1189) * DOCSP-51959 tryNext resumeToken error warning * fix link * bracket removal * move warning (cherry picked from commit 7baddf7)
…al iteration examples (mongodb#1189) * DOCSP-51959 tryNext resumeToken error warning * fix link * bracket removal * move warning (cherry picked from commit 7baddf7)
…al iteration examples (mongodb#1189) * DOCSP-51959 tryNext resumeToken error warning * fix link * bracket removal * move warning (cherry picked from commit 7baddf7)
…al iteration examples (mongodb#1189) * DOCSP-51959 tryNext resumeToken error warning * fix link * bracket removal * move warning (cherry picked from commit 7baddf7)
…al iteration examples (mongodb#1189) * DOCSP-51959 tryNext resumeToken error warning * fix link * bracket removal * move warning (cherry picked from commit 7baddf7)
…al iteration examples (mongodb#1189) * DOCSP-51959 tryNext resumeToken error warning * fix link * bracket removal * move warning (cherry picked from commit 7baddf7)
…al iteration examples (mongodb#1189) * DOCSP-51959 tryNext resumeToken error warning * fix link * bracket removal * move warning (cherry picked from commit 7baddf7)
…al iteration examples (mongodb#1189) * DOCSP-51959 tryNext resumeToken error warning * fix link * bracket removal * move warning (cherry picked from commit 7baddf7)
…al iteration examples (mongodb#1189) * DOCSP-51959 tryNext resumeToken error warning * fix link * bracket removal * move warning (cherry picked from commit 7baddf7)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-51959
Staging Links
Self-Review Checklist