Bloc Internal Mapping to Transition #840
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
felangel
added
enhancement candidate
felangel
changed the title
felangel
force-pushed
the
bloc/transition-function
branch
from
533fb8d
to
940e53b
Compare
Codecov Report
@@ Coverage Diff @@
## master #840 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 23 20 -3
Lines 373 351 -22
=========================================
- Hits 373 351 -22
Continue to review full report at Codecov.
|
felangel
force-pushed
the
bloc/transition-function
branch
2 times, most recently
from
61c7a01
to
d2856d0
Compare
felangel
force-pushed
the
bloc/transition-function
branch
from
d2856d0
to
eb71764
Compare
|
@brianegan when you have a min can you please take a look and lmk what you think? Thanks! |
jorgecoca
previously approved these changes
Feb 3, 2020
brianegan
previously approved these changes
Feb 8, 2020
There was a problem hiding this comment.
Good work! Only thing I'd consider: Adding a test to ensure each transition delivers the correct event with it. A test like that should fail in certain cases with the previous implementation (such as using flatMap instead of asyncExpand in the transformEvents function), but should work with this new implementation.
felangel
force-pushed
the
bloc/transition-function
branch
2 times, most recently
from
d5bbbb0
to
bff6ffc
Compare
felangel
force-pushed
the
bloc/transition-function
branch
from
bff6ffc
to
8c52bf5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Status
READY
Breaking Changes
YES
Description
Transitionobjects which encapsulate the current transition in order to ensure that transitions occur for the respective event/state pair -- addresses Remove RxDart dependency #839 (comment).transformStatestotransformTransitionstransformEventssignature change to take aTransitionFunctionTodos
Impact to Remaining Code Base
transformStates->transformTransitionstransformEvents