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-3726): add optional option overloads of Db's createCollection function #3019

Merged
merged 1 commit into from
Nov 2, 2021
Merged

fix(NODE-3726): add optional option overloads of Db's createCollection function #3019

merged 1 commit into from
Nov 2, 2021

Conversation

skrtheboss
Copy link
Contributor

@skrtheboss skrtheboss commented Oct 28, 2021

Description

Add support for optional options in createColelction.

What is changing?

This adds support for calling createCollection with optional options.
This is useful if for example a wrapper function is used.

Is there new documentation needed for these changes? No.

What is the motivation for this change?

Overloads do not allow optional options.

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

@skrtheboss skrtheboss changed the title fix: simplify overloads of Db's createCollection function Simplify overloads of Db's createCollection function Oct 28, 2021
@dariakp
Copy link
Contributor

dariakp commented Oct 28, 2021

@skrtheboss Hi there, thank you for submitting this suggestion. Could you clarify what issue you are running into? Currently, optional options do appear to be supported: for a promise, you can use createCollection('name') and for callback, createCollection('name', () => { // handler }).

@dariakp dariakp added the External Submission PR submitted from outside the team label Oct 28, 2021
This adds support for calling createCollection with optional options.
This is useful if for example a wrapper function is used.
@skrtheboss
Copy link
Contributor Author

HI @dariakp .
Thanks for the hint. I have updated the commit message and have added type tests.
Let me know if i implemented it correctly.

@skrtheboss skrtheboss changed the title Simplify overloads of Db's createCollection function fix: simplify overloads of Db's createCollection function Oct 29, 2021
@dariakp dariakp changed the title fix: simplify overloads of Db's createCollection function fix(NODE-3726): add optional option overloads of Db's createCollection function Oct 29, 2021
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.

Thanks for adding the tests to demonstrate your use case, we'll take a look at the changes in detail soon; meanwhile, I have created this ticket to track the PR: https://jira.mongodb.org/browse/NODE-3726

@dariakp dariakp added tracked-in-jira Ticket filed in MongoDB's Jira system Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Oct 29, 2021
@dariakp dariakp added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Nov 2, 2021
@dariakp dariakp requested a review from nbbeeken November 2, 2021 19:58
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 this!

@dariakp dariakp merged commit cbee3c5 into mongodb:main Nov 2, 2021
dariakp pushed a commit that referenced this pull request Nov 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
External Submission PR submitted from outside the team Team Review Needs review from team tracked-in-jira Ticket filed in MongoDB's Jira system
Projects
None yet
3 participants