feat: change api to async/await #37
Conversation
ghost
assigned 
There was a problem hiding this comment.
Working with Promises I always find it hard to make sure errors are really bubbled up correctly. Could you please add more tests for failure scenarios? It seems a bit hard to do as the hashing won't fail in many reasons, but at least one case I could think of is using a non supported hash function. Perhaps you could think of more cases.
As the API is so small it would be great if you could expand the BREAKING CHANGE notice on the commit message with an example on how the API changes. I think we should do this for all breaking changes, so that the changelog contains the upgrade path if you want to use the new version.
|
Really great review, @alanshaw 👏🏽👏🏽👏🏽 Lot's of knowledge shared on how to better use async/await. Thank you :) |
Address comments on [parent PR](#37)
hugomrdias
force-pushed
the
feat/async-iterators
branch
from
7233f53 to
fa17918
Compare
Address comments on [parent PR](#37)
benchmarks/hash.js
Outdated
| list.push(res) | ||
| d.resolve() | ||
| }) | ||
| multihashing(buf, alg) |
There was a problem hiding this comment.
change this to use await instead?
| sha.sha2256((Buffer.from(firstHash)), cb) | ||
| }) | ||
| default: | ||
| throw new TypeError(`${algorithm} is not a supported algorithm`) |
There was a problem hiding this comment.
I think this should be a regular Error...
src/sha.browser.js
Outdated
| return Buffer.from(await crypto.subtle.digest({ name: 'SHA-256' }, d)) | ||
| } | ||
| default: | ||
| throw new TypeError(`${algorithm} is not a supported algorithm`) |
src/sha.js
Outdated
| return crypto.createHash('sha256').update(first).digest() | ||
| } | ||
| default: | ||
| throw new TypeError(`${algorithm} is not a supported algorithm`) |
|
Lets get this released @hugomrdias! |
BREAKING CHANGE: callbacks are not supported!!
Address comments on [parent PR](#37)
hugomrdias
force-pushed
the
feat/async-iterators
branch
from
46e961d to
d6889e9
Compare
BREAKING CHANGE: callbacks are not supported!!