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

docs: db.close no longer necessary #8333

Merged
merged 4 commits into from
Feb 24, 2022
Merged

docs: db.close no longer necessary #8333

merged 4 commits into from
Feb 24, 2022

Conversation

rick-shar-ww
Copy link
Contributor

The close method is no longer on the db and the cleanup is handled by connection.close

Summary

Test plan

The `close` method is no longer on the `db` and the cleanup is handled by `connection.close`
@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@codecov-io
Copy link

Codecov Report

Merging #8333 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #8333   +/-   ##
=======================================
  Coverage   62.17%   62.17%           
=======================================
  Files         266      266           
  Lines       10694    10694           
  Branches     2603     2603           
=======================================
  Hits         6649     6649           
  Misses       3459     3459           
  Partials      586      586

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2db8852...f359d63. Read the comment docs.

Copy link
Contributor

@jeysal jeysal left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!
Ref https://mongodb.github.io/node-mongodb-native/api-generated/mongoclient.html#close (not actually related to mongodb-memory-server)

Copy link
Contributor

@jeysal jeysal left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!
Ref https://mongodb.github.io/node-mongodb-native/api-generated/mongoclient.html#close (not actually related to mongodb-memory-server)

@rick-shar-ww rick-shar-ww changed the title new version of mongodb-memory-server db.close no longer necessary Apr 16, 2019
@rick-shar-ww
Copy link
Contributor Author

LGTM, thanks!
Ref https://mongodb.github.io/node-mongodb-native/api-generated/mongoclient.html#close (not actually related to mongodb-memory-server)

I updated the name of the PR. Thanks.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

Please update the versioned docs as well

image

docs/MongoDB.md Outdated
@@ -115,7 +115,6 @@ beforeAll(async () => {

afterAll(async () => {
await connection.close();
await db.close();
Copy link
Member

Choose a reason for hiding this comment

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

we should keep a comment around saying if you're on version x, you need to call close as well

@SimenB
Copy link
Member

SimenB commented Apr 18, 2019

/cc @vladgolubev probably relevant for you

@vladholubiev
Copy link
Contributor

Thanks @rick-shar-ww! I've updated my preset at https://github.com/shelfio/jest-mongodb which is now referenced in the docs. You can commit your change again after pulling the latest master

@SimenB
Copy link
Member

SimenB commented Nov 9, 2019

@rick-shar-ww ping 🙂

@thymikee
Copy link
Collaborator

thymikee commented May 2, 2020

@rick-shar-ww still interested in this PR? 🙂

@mrazauskas mrazauskas mentioned this pull request Nov 3, 2021
This was referenced Feb 24, 2022
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

thanks!

(almost 3 years later 🙈)

@SimenB SimenB changed the title db.close no longer necessary docs: db.close no longer necessary Feb 24, 2022
@SimenB SimenB merged commit 4dc9db2 into jestjs:main Feb 24, 2022
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants