Skip to content
This repository has been archived by the owner on Jun 15, 2023. It is now read-only.

refactor: use async/await instead of callbacks #37

Merged
merged 10 commits into from
Aug 16, 2019

Conversation

jacobheun
Copy link
Contributor

@jacobheun jacobheun commented Aug 13, 2019

BREAKING CHANGE: The api now uses async/await instead of callbacks.

  • Updates all dependencies to their latest async versions
  • Adds node 12 to travis
  • Adds webworkers to the test suite
  • Updates the readme api, and cleans up some links
  • Removes pre push test, but keeps lint

@jacobheun jacobheun force-pushed the refactor/async-await branch from bbfb00c to fff701a Compare August 13, 2019 16:41
@jacobheun jacobheun marked this pull request as ready for review August 13, 2019 18:18
src/cms.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Member

@achingbrain achingbrain left a comment

Choose a reason for hiding this comment

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

Some minor nits but LGTM

chore: move promisify to dev dep
@jacobheun
Copy link
Contributor Author

👍 nits fixed

package.json Show resolved Hide resolved
*/
async function findAsync (array, asyncCompare) {
const promises = array.map(asyncCompare)
const results = await Promise.all(promises)
Copy link
Member

Choose a reason for hiding this comment

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

We have to wait for all these promises to resolve before we can find the value. This is a change in behaviour from async/detect which executes in parallel but will call the callback _as soon as _ one returns true.

...we can leave this for a separate PR.

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

Looks good! Just pointed some minor stuff mostly regarding jsdocs

src/keychain.js Outdated Show resolved Hide resolved
src/keychain.js Outdated Show resolved Hide resolved
src/keychain.js Outdated Show resolved Hide resolved
src/keychain.js Outdated Show resolved Hide resolved
src/keychain.js Outdated Show resolved Hide resolved
src/keychain.js Show resolved Hide resolved
src/keychain.js Outdated Show resolved Hide resolved
src/keychain.js Outdated Show resolved Hide resolved
src/keychain.js Outdated
const kid = await privateKey.id()
// if (err) return throwDelayed(callback, err)
const pem = await privateKey.export(this._())
// if (err) return throwDelayed(callback, err)
Copy link
Member

Choose a reason for hiding this comment

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

Should we remove all the above comments?

We should probably add a try...catch from the const kid line until the commit throwing using the throwDelayed if we catch any error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I also wrapped renameKey to make sure we're delaying there as well.

src/keychain.js Outdated Show resolved Hide resolved
jacobheun and others added 2 commits August 16, 2019 12:51
Co-Authored-By: Vasco Santos <vasco.santos@moxy.studio>
@jacobheun jacobheun requested a review from vasco-santos August 16, 2019 11:01
@vasco-santos vasco-santos merged commit dda315a into master Aug 16, 2019
@vasco-santos vasco-santos deleted the refactor/async-await branch August 16, 2019 11:12
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.

4 participants