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

Merge Store and FeedbackLoop API. Rename it to Loop. #7

Merged
merged 6 commits into from
May 15, 2020

Conversation

andersio
Copy link
Member

@andersio andersio commented May 12, 2020

This PR merges Store and FeedbackLoop into one API.

API renames

  • FeedbackLoop and Store is renamed to Loop.
  • Store.view(value:event:) is renamed to Loop.scoped(to:event:).
  • FeedbackLoop.init(initial:reduce:feedbacks:) is renamed to Loop.init(initial:reducer:feedbacks:).

Removed API

  • FeedbackLoop.start() and FeedbackLoop.stop() are now unavailable.
  • Store.state is now unavailable. It is replaced by Loop.context, which is a producer yielding Context<S, E>.

Implementation

Loop does type erasure by inheritance internally, so that it can be the currency type for both root loops (global stores) & their scoped views (local derived stores).

This also hides stuff like start() and stop() from the public API, that were intended for SignalProducer.feedbackLoop.

@andersio andersio added the enhancement New feature or request label May 12, 2020
@andersio andersio self-assigned this May 12, 2020
@andersio andersio requested a review from sergdort May 12, 2020 18:16
@andersio andersio removed the enhancement New feature or request label May 12, 2020
public init(
initial: State,
reducer: @escaping (inout State, Event) -> Void,
feedbacks: [Loop<State, Event>.Feedback]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just remove old feedback and have Feedback<State, Event>, I find it quite annoying to have API

static func whenSomething() -> Loop<State, Event>. Feedback vs
static func whenSomething() -> Feedback<State, Event>

¯_(ツ)_/¯

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd prefer to leave the gradual migration option on the table.

Copy link
Member Author

Choose a reason for hiding this comment

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

Alternatively, we can rename the old feedback to something else, so manual migration is required.

Copy link
Member Author

Choose a reason for hiding this comment

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

@sergdort lets do that in a separate PR, no?

@sergdort
Copy link
Contributor

Screenshot 2020-05-13 at 21 35 46

@andersio andersio merged commit f75acb4 into develop May 15, 2020
@andersio andersio deleted the anders/consolidate-api branch May 15, 2020 12:17
@andersio
Copy link
Member Author

@sergdort misconfiguration 🤦

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.

None yet

2 participants