Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

WiP: Adding callbacks to StoreConnection #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Will-W
Copy link

@Will-W Will-W commented Jan 9, 2019

flutter_redux has 3 optional callbacks to StoreConnection (onWillChange, onDidChange, and onInitialBuild). I have implemented one because I needed it, Issue #34 implies a need for another.

I'm happy to go ahead and implement all 3, but before I did I wanted to check:

  1. Are you happy with the de# principle?
  2. What would need to be done to the implementation of onDidChange in this PR to make it suitable for merging? The main thing I could see would be a test.

@codecov-io
Copy link

Codecov Report

Merging #37 into master will not change coverage.
The diff coverage is 87.5%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #37   +/-   ##
=======================================
  Coverage   85.71%   85.71%           
=======================================
  Files           1        1           
  Lines          35       42    +7     
=======================================
+ Hits           30       36    +6     
- Misses          5        6    +1
Impacted Files Coverage Δ
lib/flutter_built_redux.dart 85.71% <87.5%> (ø) ⬆️

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 6903b31...f7aac40. Read the comment docs.

@kdela
Copy link

kdela commented Mar 14, 2019

@davidmarne @Will-W Any estimations when It would be merged?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants