chore: add promise support to Transaction methods #147
Conversation
test/transaction.test.js
Outdated
| }); | ||
| }); | ||
|
|
||
| it('can return promise', function() { |
There was a problem hiding this comment.
A suggestion (I am not confident to say it would be a better testing approach :p)
We can test a promise returned by calling those chain methods like .then(), see example in juggler https://github.com/strongloop/loopback-datasource-juggler/blob/de4718d5b81d29ac23597172d8ee591617924fe0/test/transaction.test.js#L94
b-admike
force-pushed
the
chore/promisify-transaction-methods
branch
from
7b8d7f4 to
f63ff39
Compare
lib/transaction.js
Outdated
| */ | ||
| Transaction.prototype.commit = function(cb) { | ||
| cb = cb || createPromiseCallback(); | ||
| return this.connector.commit(this.connection, cb); |
There was a problem hiding this comment.
I think you need to return cb.promise.
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks @raymondfeng 🙇 that is correct.
b-admike
force-pushed
the
chore/promisify-transaction-methods
branch
2 times, most recently
from
069d9a8 to
494b260
Compare
test/transaction.test.js
Outdated
| }; | ||
|
|
||
| return expect( | ||
| Transaction.begin(connectorObject, '')).to.eventually.be.instanceOf(Transaction); |
There was a problem hiding this comment.
I think we are usually using slightly different formatting.
LB3 style:
return expect(Transaction.begin(connectorObject, ''))
.to.eventually.be.instanceOf(Transaction);LB4 style (?)
return expect(
Transaction.begin(connectorObject, '')
).to.eventually.be.instanceOf(Transaction);There was a problem hiding this comment.
Okay I'll go with LB3 (EDIT LB4) style 👍
Add promise support to transaction methods. Co-Authored-By: Miroslav Bajtoš <mbajtoss@gmail.com>
b-admike
force-pushed
the
chore/promisify-transaction-methods
branch
from
494b260 to
d79092a
Compare
|
I've looked at the failed builds, and none of them seem related to changes in this PR. Will merge as is. |
bajtos
changed the title
Description
Add promise support to transaction class methods.
Related issues
Checklist
guide