-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add support for using promises in Cursor methods #2554
Conversation
07759ac
to
dabeb9b
Compare
I reverted this in the case I want to add more commits to that pull request. This means that currently this pull request will fail, but I'll rebase this onto master when that (possibly?) gets merged. I'll have to re-add |
Thought I'd correctly mark this as draft until the pull request this currently depends on gets reviewed. |
Adding promises in here is totally cool...need some tests for the functionality though! |
Yup I am going to get to that now! |
rad
…On Tue, Jun 22, 2021 at 10:12 AM Bluenix ***@***.***> wrote:
Yup I am going to get to that now!
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2554 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAMHIPPC4SJ5IEO2ZFM4O3TUCR6DANCNFSM45W5UCVA>
.
|
73470a5
to
66a9434
Compare
When writing tests, prettier tries to enforce the following style: it('fetch 6 when asking for 10', function (done) {
const cursor = this.pgCursor(text)
cursor
.read(10)
.then((res) => assert.strictEqual(res.length, 6))
.error((err) => assert.ifError(err))
}) When I'd expect the following instead: it('fetch 6 when asking for 10', function (done) {
const cursor = this.pgCursor(text)
cursor.read(10)
.then((res) => assert.strictEqual(res.length, 6))
.error((err) => assert.ifError(err))
}) Thoughts on this? |
66a9434
to
21e36b1
Compare
There should be tests for this now; I thought testing resolving, rejecting and multiple read() calls was sufficient. |
Removes usage of `done()` since we can end the test when we exit the function Co-Authored-By: Charmander <[email protected]>
I always just let prettier do the formatting. Easier to not even have to care at all & everything looks consistent. |
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.
Thanks for this! looks good to me.
nit: you could use let
instead of var
.
I wasn't sure if it would cause issues with scoping since there's a bit of a difference, but that should be done now @brianc |
@brianc Is there anything hindering this from getting merged? |
This is great really sorry for the delay - life is crazy 💻 🐱 I'll get a semver minor out today w/ the changes. Thank you so much! ❤️ |
Fixes #2545. This change was simply mirrored from Client.query.
I did base this pull request on #2553, expecting that to also be merged (that way no merge conflicts arise). If this is unwanted I could change that.