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

Add option to remove all documents from collections being seeding #149

Merged
merged 1 commit into from Mar 13, 2021
Merged

Add option to remove all documents from collections being seeding #149

merged 1 commit into from Mar 13, 2021

Conversation

EltonGarcia
Copy link
Contributor

Fix #148 by adding the option to remove all documents without dropping the database or collection.

@codecov
Copy link

codecov bot commented Mar 5, 2021 •
edited

Codecov Report

Merging #149 (9b0befd) into master (27ae1a8) will increase coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #149      +/-   ##
==========================================
+ Coverage   93.43%   93.50%   +0.07%     
==========================================
  Files          18       18              
  Lines         274      277       +3     
  Branches       49       50       +1     
==========================================
+ Hits          256      259       +3     
  Misses          7        7              
  Partials       11       11
Flag Coverage Δ
cli 93.75% <100.00%> (ø)
core 93.42% <100.00%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
core/src/config.ts 93.75% <ø> (ø)
cli/src/options.ts 92.15% <100.00%> (ø)
core/src/database/database.ts 85.00% <100.00%> (+2.64%) ⬆️

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 27ae1a8...9b0befd. Read the comment docs.

@EltonGarcia EltonGarcia force-pushed the remove-all-documents branch 2 times, most recently from 24e1a49 to d773494 Compare March 5, 2021 03:46
@EltonGarcia
Copy link
Contributor Author

@pkosiec could you please help with it? I don't know what is going on.

@EltonGarcia EltonGarcia force-pushed the remove-all-documents branch 4 times, most recently from a21c7cc to 9e32562 Compare March 6, 2021 22:09
@EltonGarcia
Copy link
Contributor Author

@pkosiec sorry for bothering with it, I would love to use this library but I won’t be able to do it without this feature. Can we have a new release asap, please?

Thank you.

pkosiec
pkosiec reviewed Mar 7, 2021
View reviewed changes
Copy link
Owner

@pkosiec pkosiec left a comment

Choose a reason for hiding this comment

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

Almost there! I have a few comments. Once they are resolved, we're ready to merge this 🚀

core/src/config.ts Outdated Show resolved Hide resolved
core/src/index.ts Outdated Show resolved Hide resolved
core/src/index.ts Outdated Show resolved Hide resolved
core/test/integration/index.ts Outdated Show resolved Hide resolved
core/test/integration/index.ts Show resolved Hide resolved
core/test/integration/database.ts Outdated Show resolved Hide resolved
core/test/integration/database.ts Outdated Show resolved Hide resolved
core/test/integration/database.ts Outdated Show resolved Hide resolved
@EltonGarcia EltonGarcia force-pushed the remove-all-documents branch 3 times, most recently from c381690 to 77b2c0e Compare March 8, 2021 16:08
@EltonGarcia
Copy link
Contributor Author

@pkosiec I have fixed all the issues as you requested, except the removing the test case about a non-existing collection, if I remove this test case the code coverage test will fail.

@nickpalmer
Copy link

nickpalmer commented Mar 11, 2021 •
edited

I would love to see this feature merge and ship, and would be happy to report back if it works or not. We are currently using {dropCollection: true} but we lose some custom indexes when we do that and I was just looking for a solution, and this would do it for us.

pkosiec
pkosiec approved these changes Mar 13, 2021
View reviewed changes
Copy link
Owner

@pkosiec pkosiec left a comment •
edited

Choose a reason for hiding this comment

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

LGTM - Thank you @EltonGarcia for your contribution! Great work!

@pkosiec pkosiec merged commit d435432 into pkosiec:master Mar 13, 2021
@pkosiec pkosiec added area/cli Refers to Mongo Seeding CLI area/core Refers to Mongo Seeding library area/docker-image Refers to Mongo Seeding Docker Image 🚀 enhancement New feature or request labels Mar 13, 2021
@pkosiec
Copy link
Owner

pkosiec commented Mar 13, 2021

@nickpalmer I've just published a new draft release, so you can try it even right now 🙂 https://github.com/pkosiec/mongo-seeding/releases/tag/v3.7.0-alpha.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pkosiec pkosiec pkosiec approved these changes

Assignees
No one assigned
Labels
area/cli Refers to Mongo Seeding CLI area/core Refers to Mongo Seeding library area/docker-image Refers to Mongo Seeding Docker Image 🚀 enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove all documents?
3 participants
@EltonGarcia @nickpalmer @pkosiec