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

refactor!: remove strict/callback mode from Db.collection helper #2817

Merged
merged 9 commits into from
May 28, 2021

Conversation

emadum
Copy link
Contributor

@emadum emadum commented May 25, 2021

Strict mode has been removed from the Db.collection helper,
as well as the callback argument which was required by strict mode.

Db.createCollection can be used when a collection needs to be
created explicitly.

NODE-2752

@emadum emadum changed the title refactor!: remove strict mode and callback from db.collection helper refactor!: remove strict/callback mode from db.collection helper May 25, 2021
@emadum emadum marked this pull request as ready for review May 25, 2021 18:04
@emadum emadum requested review from a team, durran, nbbeeken and dariakp and removed request for a team May 25, 2021 18:07
@emadum emadum changed the title refactor!: remove strict/callback mode from db.collection helper refactor!: remove strict/callback mode from Db.collection helper May 25, 2021
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.

All small points, I marked the skipped tests I found so to track them but maybe your reasoning for skipping and not deleting is the same for all of them?

And some docs/test suggestions

if (!options) {
options = {};
} else if (typeof options === 'function') {
throw new TypeError('The callback form of this helper has been removed.');
Copy link
Contributor

Choose a reason for hiding this comment

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

@dariakp Just making sure we catch this, depending on who merges first this is a new error to convert.
Maybe this is best as a MongoError?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will likely go in first so I'll make a note to rebase my branch and check for any added TypeError/MongoError

src/db.ts Outdated Show resolved Hide resolved
test/functional/collection.test.js Outdated Show resolved Hide resolved
test/functional/db.test.js Outdated Show resolved Hide resolved
test/functional/db.test.js Outdated Show resolved Hide resolved
src/db.ts Show resolved Hide resolved
@emadum emadum requested a review from nbbeeken May 26, 2021 15:18
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.

🚀 lgtm!

@emadum emadum merged commit 53abfe7 into 4.0 May 28, 2021
@emadum emadum deleted the NODE-2752/remove-strict-collection-option branch May 28, 2021 13:37
ljhaywar pushed a commit that referenced this pull request Nov 9, 2021
Strict mode has been removed from the `Db.collection` helper,
as well as the callback argument which was required by strict mode.

`Db.createCollection` can be used when a collection needs to be
created explicitly.

NODE-2752
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.

3 participants