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

added update functionality #153

Merged
merged 5 commits into from
May 22, 2017
Merged

added update functionality #153

merged 5 commits into from
May 22, 2017

Conversation

lgandecki
Copy link
Contributor

@lgandecki lgandecki commented May 4, 2017

Very few changes to the actual code, since all the hard work is getting done in the modifyjs package.
I have a simple test case here just to check the integration, all the heavy lifting tests are done in modifyjs package src/modify.test.js

At this point all of the mongodb examples are working, with an exception of:

$currentDate
$pull (but $pullAll works)
$push with $sort modifier

There might be more edgecases that don't work and didn't surface while going through the mongodb examples, but we/I can try to tackle them one by one. :)

Please let me know what you think.

@pubkey
Copy link
Owner

pubkey commented May 4, 2017

Hi @lgandecki thank you for the great work. I will review it at the weekend.
Please do not add build-files (dist-folder) to the PR.

await docs.update(updateObj);
}
return docs;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could possibly extract this and remove to a common function, it's very similar (the only two differences are the "action" (.update vs .remove) and the fact that update takes an argument)

Up to you - some people like to apply a rule of three :)

@drop-c
Copy link

drop-c commented May 19, 2017

Any news on this? Looks promising

@pubkey
Copy link
Owner

pubkey commented May 22, 2017

I messed up the merge. Some tests are not working. Can you reopen the PR?

@pubkey pubkey merged commit 52437ce into pubkey:master May 22, 2017
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