-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Refactor all BbPromise.x usage which doesn't have native counterparts #8369
Comments
BbPromise.try
wraps
I created a branch to start chipping away at this. I refactored everything under I can submit the pull request once my PR for #8368 has been merged. I used the @medikoo - let me know if my work is a little premature. I actually have a good amount of experience refactoring Promises and BBPromises to async/await, so this stuff shouldn't take me too too long. Hope this stuff helps |
@ifitzsimmons great thanks for help! Ideally if we address it step by step, as outlined in issues. First by addressing #8368, and we have a lot to cover over there. Have you checked that issue? |
@medikoo I did. I made the async tagging updates for #8368 and am just waiting for it to be merged. Do you want to complete ALL of #8368 before refactoring the |
Yes, ideally if that step is finalized entirely before moving with next steps |
Create #11797 to refactor So we can safely cross them out in the original comment |
@sleepwithcoffee thanks, I've updated the description. Concerning this scope of work, ideally if first we cover #8368. Intentionally this async refactor, was planned into multiple carefully ordered steps - so we do not introduce some regressions (not all of the affected parts are well tested) |
hi @medikoo I think this was closed by accident, let's reopen this ticket |
@sleepwithcoffee thanks for heads up. Just a note. Ideally if we cover #8368 entirely first |
Use case description
Partially covers #7171
Should be addressed after #8368
Bluebird exposes a lot of functions and methods that have no standard counterparts, we should refactor it to native promises syntax
Proposed solution
Refactor to async/await all functions that contain usage of following methods:
Note: To avoid large, problematic to handle PR's. let's replace each function with different PR
BbPromise.bind
- Most likely, in all used cases, bind wrapper can be just removed (no counterpart needed)BbPromise.delay
- Timeout could be implemented with help oftimers-ext/promise/sleep
BbPromise.each
- We can simply usefor (..) { await .. }
constructBbPromise.fromCallback
- Use case for which it's used, it's probably no longer actual (if it is, we need to come up withnew Promise(..)
construct)BbPromise.join
- Replace withPromise.all
BbPromise.map
- Replace withPromise.all
BbPromise.mapSeries
- We can simply usefor (..) { await .. }
constructBbPromise.promisify
- Whenfs
functions are wrapped, resort tofs.promises
. In other cases userequire('util').promisify
BbPromise.promisifyAll
- Whenfs
functions are wrapped, resort tofs.promises
. In other cases userequire('util').promisify
BbPromise.props
- Refactor withPromise.all
BbPromise.reduce
- We can simply usefor (..) { await .. }
constructBbPromise.try
- As all functions containing it are now declared as async, this wrapper can simply be just removedThe text was updated successfully, but these errors were encountered: