Skip to content
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

feat: remove dependence on Bluebird and delay #67 #75

Merged
merged 2 commits into from Nov 22, 2019

Conversation

@jbr
Copy link
Contributor

jbr commented Nov 21, 2019

This approach uses a simple setTimeout-based delay in the only production
usage (src/classes/queue-events.ts), defined in src/util.ts, and since
there was already a devDependency on a library called
delay, this commit uses that in
test contexts. It should be easy to replace that dep with a
(ms:number) => new Promise(r => setTimeout(r, ms)) as well.

Closes #67

This approach uses a simple setTimeout-based delay in the only production
usage (src/classes/queue-events.ts), defined in src/util.ts, and since
there was already a devDependency on a library called
[delay](https://www.npmjs.com/package/delay), this commit uses that in
test contexts. It should be easy to replace that dep with a
`(ms:number) => new Promise(r => setTimeout(r, ms))` as well.

Closes #67
Copy link
Contributor

manast left a comment

Looks good. One thing, why not using the delay in utils for the tests as well and we can get rid of another dependency?

this commit uses delay from utils in typescript and adds a simple
test/fixtures/delay.js for use in javascript fixtures. this change also
allowed the ts compiler to identify a promise that was not `await`ed in
src/test/test_worker.ts
@jbr jbr changed the title feat: remove dependence on Bluebird.delay #67 feat: remove dependence on Bluebird and delay #67 Nov 22, 2019
@@ -0,0 +1,5 @@
module.exports = function delay(ms) {

This comment has been minimized.

Copy link
@manast

manast Nov 22, 2019

Contributor

why cant we use the delay implementation in utils?

This comment has been minimized.

Copy link
@jbr

jbr Nov 22, 2019

Author Contributor

That would be requiring ts from js, which seemed like a substantial change to the fixtures

This comment has been minimized.

Copy link
@manast

manast Nov 22, 2019

Contributor

ah ok, got it.

@manast
manast approved these changes Nov 22, 2019
@manast manast merged commit 8d38bfa into taskforcesh:master Nov 22, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on feat-67-remove-bluebird-dependency at 87.551%
Details
@manast

This comment has been minimized.

Copy link
Contributor

manast commented Nov 22, 2019

🎉 This PR is included in version 1.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@manast manast added the released label Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.