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.

Sign up for GitHub

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

Jump to bottom

fix(NODE-2370): correct a return type of hasNext() #3058

Merged
merged 2 commits into from
Nov 29, 2021
Merged

fix(NODE-2370): correct a return type of hasNext() #3058

merged 2 commits into from
Nov 29, 2021

Conversation

wegylexy
Copy link
Contributor

Description

hasNext() returns Promise<boolean> instead of Promise<void>. The type definition is wrong.

This initial pull request fixes one instance. There are more.

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: <type>(NODE-xxxx)<!>: <description>
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

nbbeeken
nbbeeken requested changes Nov 29, 2021
View reviewed changes
Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

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

Thanks for the help, always appreciated! I have a small suggestion on how we can make the typing a bit more useful.

nbbeeken
nbbeeken requested changes Nov 29, 2021
View reviewed changes
src/change_stream.ts Show resolved Hide resolved
@nbbeeken nbbeeken added the External Submission PR submitted from outside the team label Nov 29, 2021
@wegylexy
Copy link
Contributor Author

If somebody has time, please fix other hasNext() too.

nbbeeken
nbbeeken approved these changes Nov 29, 2021
View reviewed changes
Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

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

Look like the hasNext on cursor is already in the correct format, so we're almost good to go here. Thanks again!

@nbbeeken nbbeeken added the Team Review Needs review from team label Nov 29, 2021
dariakp
dariakp approved these changes Nov 29, 2021
View reviewed changes
Copy link
Contributor

@dariakp dariakp left a comment

Choose a reason for hiding this comment

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

LGTM

@nbbeeken nbbeeken merged commit b6a63df into mongodb:main Nov 29, 2021
@wegylexy wegylexy deleted the patch-1 branch February 11, 2022 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nbbeeken nbbeeken nbbeeken approved these changes

@dariakp dariakp dariakp approved these changes

Assignees
No one assigned
Labels
External Submission PR submitted from outside the team Team Review Needs review from team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wegylexy @nbbeeken @dariakp