Skip to content
This repository has been archived by the owner on Sep 9, 2021. It is now read-only.

refactor: async iterators #25

Merged
merged 4 commits into from
Apr 13, 2019
Merged

refactor: async iterators #25

merged 4 commits into from
Apr 13, 2019

Conversation

alanshaw
Copy link
Member

@alanshaw alanshaw commented Nov 30, 2018

Here's a proposal for the datastore interface that uses async/await and async iterators.

screenshot 2018-11-30 at 12 41 03 😄

See ipfs/js-ipfs#1670 for context.

BTW I have no clue about flow types and have probably broken everything...I'll address that later if this PR gets a 👍

cc @ipfs/javascript-team

Here's a proposal for the datastore interface that uses async/await and async iterators.

See ipfs/js-ipfs#1670 for context.

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
Alan Shaw added 2 commits November 30, 2018 12:18
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
Copy link
Contributor

@jacobheun jacobheun left a comment

Choose a reason for hiding this comment

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

Just some minor feedback in the tests. Overall the API changes look good and should be straightforward to change for the other datastore's.

test/utils.spec.js Outdated Show resolved Hide resolved
test/utils.spec.js Show resolved Hide resolved
@hugomrdias
Copy link
Member

hugomrdias commented Nov 30, 2018

this looks amazing !! love the -522 :)

On a side note i have been looking into https://github.com/sithmel/iter-tools for iterator utils.

It looks pretty good and complete with 1 minor problem the packaging.

The author uses TS and does lots of builds es5 cjs, es5 esm, es2018 etc we would use it like const map = require('iter-tools/es2018/map) in the current setup. But i opened and issue iter-tools/iter-tools#77 to hopefully improve this problem and allow for const map = require('iter-tools/map)

ps. const map = require('iter-tools/map) works now but node and browser would run totally not optimal code.

Give it a look and tell me what you think.

@alanshaw
Copy link
Member Author

I was going to mention that we should probably settle on a tool like that...although I'm loving how easy those tools are to implement.

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
alanshaw pushed a commit to ipfs/js-datastore-level that referenced this pull request Dec 2, 2018
Uses async await and async iterators to implement the proposal here ipfs/interface-datastore#25

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
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.

LGTM!
-522 😁

@daviddias
Copy link
Member

@vasco-santos it is just minus 199

image

That said, still great!

@daviddias daviddias requested a review from pgte January 27, 2019 10:38
Copy link
Contributor

@pgte pgte left a comment

Choose a reason for hiding this comment

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

Much cleaner API, great work on the docs and tests!

@pgte
Copy link
Contributor

pgte commented Jan 28, 2019

@alanshaw please tell me when you need a release.

@achingbrain
Copy link
Member

Can this be released?

@daviddias daviddias merged commit ab2f2b9 into master Apr 13, 2019
@ghost ghost removed the status/in-progress In progress label Apr 13, 2019
@daviddias daviddias deleted the refactor/async-iterators branch April 13, 2019 17:04
@daviddias
Copy link
Member

Tried but got:

[10:08:11] Test [completed]
[10:08:11] Bump Version [started]
[10:08:11] Bump Version: v0.6.0 -> v0.7.0 [title changed]
[10:08:11] Bump Version: v0.6.0 -> v0.7.0 [completed]
[10:08:11] Build [started]
[10:08:11] Clean ./dist [started]
[10:08:11] Clean ./dist [completed]
[10:08:11] Webpack Build [started]
[10:08:11] Webpack Build (94.68 KB) [title changed]
[10:08:11] Webpack Build (94.68 KB) [completed]
[10:08:11] Minify [started]
[10:08:12] Minify [failed]
[10:08:12] → Unexpected token name «await», expected punc «(»
[10:08:12] Build [failed]
[10:08:12] → Unexpected token name «await», expected punc «(»
Unexpected token name «await», expected punc «(»
SyntaxError: Unexpected token name «await», expected punc «(»
    at JS_Parse_Error.get (eval at <anonymous> (/Users/dourada/code/interface-datastore/node_modules/uglify-es/tools/node.js:21:1), <anonymous>:75:23)
    at onError (/Users/dourada/code/interface-datastore/node_modules/aegir/src/error-handler.js:11:30)
npm ERR! code ELIFECYCLE

@daviddias
Copy link
Member

@alanshaw gave you publish perms
image

@achingbrain
Copy link
Member

@daviddias thanks for trying. It needs the latest aegir to build successfully - #26 should do it.

@achingbrain
Copy link
Member

Also, enable 2FA! 😜

achingbrain pushed a commit to ipfs/js-datastore-level that referenced this pull request May 29, 2019
Uses async await and async iterators to implement the proposal here ipfs/interface-datastore#25

License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
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.

7 participants