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 <~@charmander.me>
I always just let prettier do the formatting. Easier to not even have to care at all & everything looks consistent. |
brianc left a comment
There was a problem hiding this comment.
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.