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

To promise #17

Closed
wants to merge 6 commits into from
Closed

To promise #17

wants to merge 6 commits into from

Conversation

haoxins
Copy link

@haoxins haoxins commented Mar 5, 2016

No description provided.

@haoxins
Copy link
Author

haoxins commented Mar 5, 2016

the first step to make chan work with ES7 async/await >_<

@haoxins
Copy link
Author

haoxins commented Mar 6, 2016

cc @brentburg

@brentropy
Copy link
Owner

Sorry, I have been slow to get to this. I will take a look. A while back a started a big overhaul that was going to be released as chan 1.0 that included migrating from thunks to promises, but I got busy and never finished. I want to compare some of your changes to mine, but I am probably good with going forward with your changes, since I don't have the time to dedicate to it right now. Thanks @coderhaoxin.

@haoxins
Copy link
Author

haoxins commented Apr 11, 2016

@brentburg Anything else I can do to help the migration?

Can you give a roadmap or some thoughts?

I want to update chan in cojs/busboy, and also some private projects (that will migrate from co to async&await).

@haoxins
Copy link
Author

haoxins commented May 12, 2016

@brentburg Can we merge this first.

So, I can update chan in co-busboy, see

cojs/busboy#33
cojs/busboy#30

this first argument. When a value is available on the channel the callback is
called with the value or error. In this case the channel itself can also be a
thunk.
Values are removed from the channel by calling it with a Promise function chain.
Copy link

@juliangruber juliangruber May 12, 2016

Choose a reason for hiding this comment

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

I'd reword this to something like Values are taken off the channel by resolving the channel promise repeatedly. You don't have to chain to get values.

@haoxins
Copy link
Author

haoxins commented Jul 1, 2016

ping @brentburg


```js
ch(function (err, val) {
// called when there is a value or error on the channel
ch.then(function (val) {
Copy link
Owner

Choose a reason for hiding this comment

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

This doesn't seem to be implemented. The channel is a function does not have a then method.

@brentropy
Copy link
Owner

Sorry, I have gone so long without doing anything with this. There seem to be some things missing at this point (i.e. the channel function isn't thenable) and looking at the test I am still seeing a lot of thunk stuff. I have a local branch I could push up (on my old laptop, so I can't right now) that was a deeper refactor to make everything promise friendly, but I still needed to rework the tests and probably fix some bugs.

If you want to push forward with your branch and fill in the gaps, that works for me, otherwise we can see what needs to be done to go forward with my branch.

@haoxins
Copy link
Author

haoxins commented Jul 4, 2016

the channel function isn't thenable, and looking at the test I am still seeing a lot of thunk stuff

Yea! This is just the first step to Promise :)

I have a local branch I could push up (on my old laptop, so I can't right now) that was a deeper refactor to make everything promise friendly

👍 @brentburg can you push this to github? So, maybe I can give some help.

@haoxins
Copy link
Author

haoxins commented Jul 4, 2016

If you want to push forward with your branch and fill in the gaps, that works for me, otherwise we can see what needs to be done to go forward with my branch.

I don't care my branch or your branch. Just want to finish that (to Promise) in a short time.

@brentropy
Copy link
Owner

@coderhaoxin see #18. It is a v1 branch and has other changes. Let me know what you think. I did a quick to-do list of what I think still needs to be done. I can try to spend some time on it too, but if you are interested in working on that branch that would be great.

@haoxins
Copy link
Author

haoxins commented Jul 7, 2016

closed by #18

@haoxins haoxins closed this Jul 7, 2016
@haoxins
Copy link
Author

haoxins commented Jul 7, 2016

@brentburg I'll check that this weekend!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants