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

Use util.promisify instead of Bluebird.promisify #3470

Merged
merged 2 commits into from Oct 15, 2019

Conversation

@maximelkin
Copy link
Contributor

maximelkin commented Oct 8, 2019

fixes #3257

@maximelkin

This comment has been minimized.

Copy link
Contributor Author

maximelkin commented Oct 9, 2019

Not sure what to do with Bluebird.promisifyAll in https://github.com/tgriesser/knex/blob/master/lib/dialects/oracle/index.js#L85

@kibertoad kibertoad closed this Oct 9, 2019
@kibertoad kibertoad reopened this Oct 9, 2019
@lorefnon

This comment has been minimized.

Copy link
Collaborator

lorefnon commented Oct 10, 2019

node-oracle hasn't been updated since 2016 and is deprecated in favor of node-oracledb. Support for it can perhaps just be dropped.

@maximelkin maximelkin closed this Oct 12, 2019
@maximelkin maximelkin reopened this Oct 12, 2019
return bluebird
.promisify(fs.stat, { context: fs })(dir)
.catch(() => bluebird.promisify(mkdirp)(dir));
return promisify(fs.stat)(dir).catch(() => promisify(mkdirp)(dir));

This comment has been minimized.

Copy link
@kibertoad

kibertoad Oct 12, 2019

Collaborator

{ context: fs } part is irrelevant?

This comment has been minimized.

Copy link
@maximelkin

maximelkin Oct 12, 2019

Author Contributor

yes, but i cant find it in official docs or changelogs
only this example
https://nodejs.org/docs/latest-v8.x/api/util.html#util_util_promisify_original

@kibertoad

This comment has been minimized.

Copy link
Collaborator

kibertoad commented Oct 13, 2019

Bunch of conflicts came up.

@maximelkin maximelkin force-pushed the maximelkin:bluebird_remove_promisify branch from 589025e to 59bf421 Oct 14, 2019
@kibertoad kibertoad merged commit 5317148 into knex:master Oct 15, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kibertoad

This comment has been minimized.

Copy link
Collaborator

kibertoad commented Oct 15, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.