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

implement findAndModify #211

Merged
merged 5 commits into from
Jul 21, 2021
Merged

implement findAndModify #211

merged 5 commits into from
Jul 21, 2021

Conversation

DerNamenlose
Copy link
Contributor

I need the findAndModify operation for a project, so I figured I'd try and contribute it to the driver.

This also adds a few interfaces to lock down the interface in terms of types, that could be re-used in other existing interfaces like findOne, although I didn't change those yet.

src/types.ts Outdated
*/
export interface FindAndModifyOptions<T> {
/**
* Control the order in which documents are found.
Copy link
Member

Choose a reason for hiding this comment

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

align those comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Those must've slipped under the radar.

@manyuanrong
Copy link
Member

LGTM, a little formatting problem

@manyuanrong
Copy link
Member

I forgot something, can you add a test for the new method?

@DerNamenlose
Copy link
Contributor Author

I'll give it a go. It'll probably take a few days, because I'll have to figure out the best way to to that first.

@DerNamenlose
Copy link
Contributor Author

OK, that was quicker, than expected. I added two test cases, one for the update and one for the delete case.

@manyuanrong
Copy link
Member

@DerNamenlose ci failed

@DerNamenlose
Copy link
Contributor Author

Fixed. Funny enough: my deno fmt --check didn't find those initially. Must've called it wrong or something.

@manyuanrong
Copy link
Member

Fixed. Funny enough: my deno fmt --check didn't find those initially. Must've called it wrong or something.

Maybe it's a problem with the Deno version

@manyuanrong manyuanrong merged commit a05e014 into denodrivers:main Jul 21, 2021
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.

2 participants