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.

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

fix(NODE-3510): omit incorrect | void in declaration of Promise overload of rename() #2922

Merged
merged 1 commit into from
Aug 2, 2021

Conversation

justingrant
Copy link
Contributor

Description

One of the TS overloaded declarations for rename() had a | void which AFAIK is incorrect. If a method returns a Promise, it should never be typed | void. The problem doesn't show up in any other overload declarations, so I assume this was just a bug, not a statement about weird behavior of this method.

What changed?

I removed the | void for this overload's declaration.

Are there any files to ignore?

No.

One of the TS overloaded declarations for `rename()` had a `| void` which AFAIK is incorrect. If a method returns a `Promise`, it should never be typed `| void`.
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 catching this! it was certainly a mistake, only the callback overloads should return void. LGTM

@nbbeeken nbbeeken added the Team Review Needs review from team label Aug 2, 2021
@dariakp dariakp changed the title Omit incorrect | void in declaration of Promise overload of rename() fix: Omit incorrect | void in declaration of Promise overload of rename() Aug 2, 2021
@nbbeeken nbbeeken changed the title fix: Omit incorrect | void in declaration of Promise overload of rename() fix(NODE-3510): omit incorrect | void in declaration of Promise overload of rename() Aug 2, 2021
@nbbeeken nbbeeken merged commit 58c1e84 into mongodb:4.0 Aug 2, 2021
ljhaywar pushed a commit that referenced this pull request Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants