Skip to content

Conversation

rustagir
Copy link
Contributor

@rustagir rustagir commented Feb 6, 2023

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-26099
Staging - https://docs-mongodbcom-staging.corp.mongodb.com/node/docsworker-xlarge/DOCSP-26099-cs-iterable/usage-examples/changeStream/

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?

@rustagir
Copy link
Contributor Author

rustagir commented Feb 6, 2023

I'm divided on whether to update the example to reflect this change and use a for-await loop in the code snippet, instead of a listener.

Copy link
Collaborator

@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.

just one small suggestion!

@rustagir rustagir requested a review from mongoKart February 7, 2023 20:28
Copy link
Collaborator

@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.

Looks better. A few more small suggestions

TypeScript specific features of the driver relevant to this use case.

If you run the preceding example, you should see the following output:
When you run this code and then make any changes to the ``haikus``
Copy link
Collaborator

Choose a reason for hiding this comment

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

I: Next clause refers to a single change

S:

Suggested change
When you run this code and then make any changes to the ``haikus``
When you run this code and then make a change to the ``haikus``

print change events that occur on the collection.

First, open the change stream on the collection and then define a listener
on the change stream using the ``on()`` method. Once set, generate a change
Copy link
Collaborator

Choose a reason for hiding this comment

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

I: "Once set" is vague

S: "Once the [whatever] is [whatever], generate a change event..."

Comment on lines 180 to 181
To generate the change event on the collection, let's use ``insertOne()``
method to add a new document. Since the ``insertOne()`` may run before the
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
To generate the change event on the collection, let's use ``insertOne()``
method to add a new document. Since the ``insertOne()`` may run before the
To generate the change event on the collection, let's use the ``insertOne()``
method to add a new document. Since ``insertOne()`` may run before the

Comment on lines 185 to 188
We also use ``simulateAsyncPause`` after the insertion of the document
to provide ample time for the listener function to receive the change
event and for the listener to complete its execution before
closing the ``ChangeStream`` instance using the ``close()`` method.
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
We also use ``simulateAsyncPause`` after the insertion of the document
to provide ample time for the listener function to receive the change
event and for the listener to complete its execution before
closing the ``ChangeStream`` instance using the ``close()`` method.
We also use ``simulateAsyncPause`` after the insertion of the document.
This provides ample time for the listener function to receive the change
event and for the listener to complete its execution before
closing the ``ChangeStream`` instance using the ``close()`` method.

Comment on lines 190 to 192
The timers used in this example are only necessary for this demonstration
to make sure there is enough time to register listener and have the
listener process the event before exiting.
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: Put this in a Note admonition to make it stand out

Suggested change
The timers used in this example are only necessary for this demonstration
to make sure there is enough time to register listener and have the
listener process the event before exiting.
.. note:: You don't need timers to use listener functions.
The timers in this code example are for demonstration purposes only.

...or something similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this creates two stacked notes, but the second note is repetitive from the previous example. I think I should either shift the content out of the note back to normal text, or delete the repetitive note about connecting to MongoDB/sample data.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm in favor of deleting the redundant note.

@rustagir rustagir requested a review from mongoKart February 8, 2023 14:59
Copy link
Collaborator

@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 + removing redundant note

@rustagir rustagir merged commit 9086828 into mongodb:master Feb 8, 2023
rustagir added a commit to rustagir/docs-node that referenced this pull request Feb 8, 2023
* DOCSP-26099: change stream async iterable

* addtl work

* MW PR fixes 1

* small fixes

* MW PR fixes 2

* small fixes

* remove redundant note

(cherry picked from commit 9086828)
rustagir added a commit to rustagir/docs-node that referenced this pull request Feb 8, 2023
* DOCSP-26099: change stream async iterable

* addtl work

* MW PR fixes 1

* small fixes

* MW PR fixes 2

* small fixes

* remove redundant note

(cherry picked from commit 9086828)
rustagir added a commit to rustagir/docs-node that referenced this pull request Feb 8, 2023
* DOCSP-26099: change stream async iterable

* addtl work

* MW PR fixes 1

* small fixes

* MW PR fixes 2

* small fixes

* remove redundant note

(cherry picked from commit 9086828)
rustagir added a commit to rustagir/docs-node that referenced this pull request Feb 8, 2023
* DOCSP-26099: change stream async iterable

* addtl work

* MW PR fixes 1

* small fixes

* MW PR fixes 2

* small fixes

* remove redundant note

(cherry picked from commit 9086828)
@rustagir
Copy link
Contributor Author

rustagir commented Feb 8, 2023

💚 All backports created successfully

Status Branch Result
v5.0
v4.14
v4.13
v4.12

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

rustagir added a commit that referenced this pull request Feb 8, 2023
* DOCSP-26099: change stream async iterable

* addtl work

* MW PR fixes 1

* small fixes

* MW PR fixes 2

* small fixes

* remove redundant note

(cherry picked from commit 9086828)
rustagir added a commit that referenced this pull request Feb 8, 2023
* DOCSP-26099: change stream async iterable

* addtl work

* MW PR fixes 1

* small fixes

* MW PR fixes 2

* small fixes

* remove redundant note

(cherry picked from commit 9086828)
rustagir added a commit that referenced this pull request Feb 8, 2023
* DOCSP-26099: change stream async iterable

* addtl work

* MW PR fixes 1

* small fixes

* MW PR fixes 2

* small fixes

* remove redundant note

(cherry picked from commit 9086828)
rustagir added a commit that referenced this pull request Feb 8, 2023
* DOCSP-26099: change stream async iterable

* addtl work

* MW PR fixes 1

* small fixes

* MW PR fixes 2

* small fixes

* remove redundant note

(cherry picked from commit 9086828)
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