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

Various improvements, some of which actually fix a test #875

Merged
merged 3 commits into from Jul 14, 2021

Conversation

maackle
Copy link
Member

@maackle maackle commented Jul 14, 2021

This is kind of a big PR to fix one test, but it took a while to figure out the problem, and all of the changes I believe are improvements:

  • make AppStatusFx (previously AppStatusTransitionEffects) #[must_use]
    • this helps ensure that any time an app state transition occurs, a followup processing of the after-effects must be done at some point to reconcile cell state with app state
  • make AppStatusFx a monoid
    • this just means that it's easy to fold over a collection of FX to get a single FX to process, which allows us to effectively process after-effects from multiple state transitions, which is especially powerful when combined with must_use
  • reorders the registration of dnas, in particular the "extra dnas" that get added by SweetConductor

Copy link
Contributor

@neonphog neonphog left a comment

Choose a reason for hiding this comment

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

lgtm

@maackle maackle merged commit 75cbf28 into develop Jul 14, 2021
@maackle maackle deleted the fix-paused-app-restart-test branch July 14, 2021 20:37
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