-
Notifications
You must be signed in to change notification settings - Fork 216
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
Replace count by estimatedDocumentCount #155
base: master
Are you sure you want to change the base?
Conversation
index.js
Outdated
@@ -51,7 +51,7 @@ function paginate(query, options, callback) { | |||
} | |||
promises = { | |||
docs: docsQuery.exec(), | |||
count: this.count(query).exec() | |||
count: this.estimatedDocumentCount(query).exec() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, change this to countDocuments(query)
as estimatedDocumentCount
doesn't accept a query and doesn't guarantee accurate count, this is causing tests to fail and blocking the merge.
Is there any reason why this PR is not going forward? |
hi guys, any news about this issue?? |
Well, PR author gave me thumbs up ages ago, but for some reason he won't update the PR 🤷♂️ |
Sorry for the delay guys. |
Hi @AmrSaber, I think this PR has been updated; kindly check it out. |
@JosephmBassey, I reviewed the update and unfortunately, I don't know why tests are failing this time. |
I will check it out. |
Any update on this, it seems the lock file is included, maybe this is causing the fail on CI? |
Solving the
count
deprecation warning#144
#146