search

db-migrate / mongodb

mongodb driver for db-migrate
Other
25 stars 58 forks source link

ADD: removeMany case to _run function (internal) #7

Open meengit opened 8 years ago

meengit commented 8 years ago

I couldn't delete multiple documents. I know _run is private, but still thought it would be helpful to suggest to fix it:

If you had run till now with something like: db._run('remove', 'collectionName', { myKey: 'string' }); you couldn't remove many documents.

The code to delete many implied to use it as: db._run('remove', 'collectionName', [{ myKey: 'string' }]); (check for util.isArray(options)) but that would throw an error [1].

To keep current code running and not to break stuff I introduced the removeMany: db._run('removeMany', 'collectionName', { myKey: 'string' })

1: db.collection.deleteMany()

What do you think? Or did you have an other idea when you checked for util.isArray(options)?

wzrdtales commented 8 years ago

I will review this a bit later, short on time currently.

marc0der commented 7 years ago

Could this be reviewed and possibly merged please? This would help me a lot in solving a problem I currently have.

meengit commented 6 years ago

Thank you very much for reviewing and commenting.

The pull request is open since

Sep 1, 2016,

or Release v1.1.4 (Feb 5, 2016). If I saw it correctly, tests were not present at this version.

In the meantime, we do not need the db-migrate anymore. However, I am interested writing the tests.

I am not familiar with the project anymore. I have to familiarize myself again. Unfortunately, I am not able to do this before Monday, Feb 19, 2018.

Please let me know if I should write.

wzrdtales commented 6 years ago

@meengit Yes this has been open for too long that is very true. If you want to write the tests for this that would be fantastic and I can just appreciate your spirit :unicorn: even though it has been open for way too long.