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

Added Parallel applicative and also breakingly... #23

Merged
merged 2 commits into from
Jun 26, 2019

Conversation

polytypic
Copy link
Member

@polytypic polytypic commented Oct 4, 2018

...changed template and apply to work in parallel and renamed
[upH|downH|h]asFailed to [upH|downH|h]asErrored.

@polytypic polytypic added enhancement New feature or request breaking labels Oct 4, 2018
@codecov-io
Copy link

codecov-io commented Oct 4, 2018

Codecov Report

Merging #23 into master will increase coverage by 0.13%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #23      +/-   ##
==========================================
+ Coverage   99.15%   99.29%   +0.13%     
==========================================
  Files           1        1              
  Lines         237      282      +45     
  Branches       26       36      +10     
==========================================
+ Hits          235      280      +45     
  Misses          2        2
Impacted Files Coverage Δ
dist/karet.xhr.es.js 99.29% <0%> (+0.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 74c0324...7fe41ac. Read the comment docs.

@polytypic polytypic force-pushed the feature/parallel-composition branch 20 times, most recently from d7c034f to e6c0038 Compare October 10, 2018 13:12
@polytypic polytypic force-pushed the feature/parallel-composition branch from e6c0038 to 8f5ab38 Compare October 12, 2018 13:45
@polytypic polytypic force-pushed the feature/parallel-composition branch 4 times, most recently from a9c6313 to 0d1c39d Compare January 2, 2019 00:06
...changed `template` and `apply` to work in parallel and renamed
`[upH|downH|h]asFailed` to `[upH|downH|h]asErrored`.
@polytypic polytypic force-pushed the feature/parallel-composition branch from 0d1c39d to 71537dd Compare January 5, 2019 09:06
@abstracthat
Copy link
Member

Hi @polytypic. I've finally reached the point of swapping out our app's API fetching layer and really need this Parallel algebra to pass to L.traverse, so that my XHR waterfall is not unecessarily serialized.

What do you feel is left before we merge this? I checked out the branch and the tests are passing. I've also added it to my work in progress branch in our app and it's just what I need. Happy to help get it shipped if you feel it needs anything else.

@polytypic
Copy link
Member Author

Hmm... I don't recall anything particular. So, maybe review once more, merge, and publish if all looks good.

@abstracthat
Copy link
Member

Sounds good. I've read through everything carefully and it looks good but I don't quite understand why we need the delayUnsub wrapper around the perform request. This causes the property to not be unsubscribed from until the next tick... do we need this for the merging the XHRs?

Any objection to releasing this as 1.0? It feels rather feature complete with these Happy path combinators. It's really pleasant to work with now that I've played with it and understand how to combine the requests with L.traverse. Swapping out the algebra is 💪

@polytypic
Copy link
Member Author

I don't quite understand why we need the delayUnsub wrapper around the perform request. This causes the property to not be unsubscribed from until the next tick... do we need this for the merging the XHRs?

Yes, otherwise the subscription would be dropped momentarily resulting in repeating the subscription effect (abort and restart request). See here.

@abstracthat
Copy link
Member

@polytypic I added apParallel to the readme since it's exported and could potentially be useful. If you are okay with the 1.0.0 version bump I'll ship this tomorrow.

@abstracthat abstracthat merged commit 52699a5 into master Jun 26, 2019
@abstracthat abstracthat deleted the feature/parallel-composition branch June 26, 2019 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants