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

Provide progress parameter via fold pending parameter #20

Merged
merged 2 commits into from
Oct 29, 2018

Conversation

mlegenhausen
Copy link
Contributor

No description provided.

fold<B>(initial: B, pending: B, failure: Function1<L, B>, success: Function1<A, B>): B {
fold<B>(
initial: B,
pending: Function1<Option<RemoteProgress>, B>,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a massive breaking change. Would foldL be enough for you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure I can split the changes and provide the breaking change in a separate pull request?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well the problem is actually deeper. If we start processing pendings with functions then we should also change pending helper which immediately turns out to be the same as progress. Smth one should be left.
/cc @sutarmin @OliverJAsh

This may be a single PR for the next breaking 0.4 version together with #5

@mlegenhausen
Copy link
Contributor Author

@raveclassic something missing?

@raveclassic
Copy link
Contributor

@mlegenhausen sorry, slipped my mind :)

@raveclassic raveclassic merged commit 613e2fb into devexperts:master Oct 29, 2018
@mlegenhausen
Copy link
Contributor Author

@raveclassic thanks

mlegenhausen pushed a commit to werk85/remote-data-ts that referenced this pull request Oct 29, 2018
raveclassic pushed a commit that referenced this pull request Nov 6, 2018
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.

2 participants