Skip to content
This repository has been archived by the owner on Sep 21, 2022. It is now read-only.

Cast result of all wd method to Bluebird promises #626

Merged
merged 1 commit into from Oct 10, 2016
Merged

Conversation

SevInf
Copy link
Contributor

@SevInf SevInf commented Oct 7, 2016

Introduce small wd-bluebird module, which just wraps existing
wd methods and cast the result to Blubird promise.

/cc @sipayRT @j0tunn @DudaGod

@j0tunn
Copy link
Contributor

j0tunn commented Oct 7, 2016

а зачем это?

@SevInf
Copy link
Contributor Author

SevInf commented Oct 7, 2016

@j0tunn wd внутри использует Q. Если промисы не скастовать, придется везде думать, какой из промисов сейчас возвращается (они не 100% совместимы по интерфейсу)

@coveralls
Copy link

coveralls commented Oct 7, 2016

Coverage Status

Coverage increased (+0.1%) to 83.853% when pulling f7a0edb on fix/wd-bluebird into fe878bb on master.

@SevInf
Copy link
Contributor Author

SevInf commented Oct 10, 2016

@j0tunn @sipayRT ping

@sipayRT
Copy link
Member

sipayRT commented Oct 10, 2016

need some tests for wd wrapper

@SevInf
Copy link
Contributor Author

SevInf commented Oct 10, 2016

@sipayRT готово

@coveralls
Copy link

coveralls commented Oct 10, 2016

Coverage Status

Coverage increased (+0.1%) to 83.846% when pulling ded8922 on fix/wd-bluebird into fe878bb on master.

});

wrapped = wd.promiseRemote();
sandbox = sinon.sandbox.create();
Copy link
Member

Choose a reason for hiding this comment

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

вынеси из beforeEach, плз - ты ведь ресторишь его в afterEach и не изменяешь по ходу тестов

});

afterEach(() => {
sandbox.restore();
Copy link
Member

Choose a reason for hiding this comment

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

в одну строчку можно

@sipayRT
Copy link
Member

sipayRT commented Oct 10, 2016

в остальном ок

sandbox.restore();
});

it('should cast original promise to Blubird', () => {
Copy link
Member

Choose a reason for hiding this comment

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

...to Bluebird


if (EventEmitter.prototype[methodName]) {
return false;
}
Copy link
Member

Choose a reason for hiding this comment

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

А почему так не написать:

if (Object.prototype[methodName] || EventEmitter.prototype[methodName]) {
    return false;
}

Copy link
Member

Choose a reason for hiding this comment

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

раздельно лучше читаются условия

exports.promiseRemote = (gridUrl) => {
const remote = wd.promiseRemote(gridUrl);

const bluebirdRemote = {};
Copy link
Member

Choose a reason for hiding this comment

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

а зачем тут пробел между переменными?

Introduce small wd-bluebird module, which just wraps existing
wd methods and cast the result to Blubird promise.
@SevInf
Copy link
Contributor Author

SevInf commented Oct 10, 2016

@sipayRT @DudaGod 🆙

@SevInf SevInf merged commit 36228ff into master Oct 10, 2016
@SevInf SevInf deleted the fix/wd-bluebird branch October 10, 2016 14:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants