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

WP-5043 Improve store state error messages #109

Merged
merged 1 commit into from
Aug 24, 2017
Merged

Conversation

georgelesica-wf
Copy link
Contributor

Problem

The message associated with the StateError that is thrown when an action subscription is managed or when the store is listened to after disposal is cryptic.

Solution

Add the runtimeType to help the situation a bit.

Potential Regressions

None.

Tests

Existing tests are sufficient, the new error message is an ergonomic issue.

QA / +10

  1. CI passes

FYI

@seanburke-wf

@aviary-wf
Copy link

aviary-wf commented Aug 24, 2017

Raven

Number of Findings: 0

@rm-astro-wf rm-astro-wf changed the title Improve store state error messages WP-5043 Improve store state error messages Aug 24, 2017
@codecov-io
Copy link

codecov-io commented Aug 24, 2017

Codecov Report

Merging #109 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #109   +/-   ##
=======================================
  Coverage   90.32%   90.32%           
=======================================
  Files           5        5           
  Lines          62       62           
=======================================
  Hits           56       56           
  Misses          6        6
Impacted Files Coverage Δ
lib/src/store.dart 90.9% <100%> (ø) ⬆️

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 1d07523...181024f. Read the comment docs.

@seanburke-wf
Copy link
Contributor

+10
CI passes

@georgelesica-wf
Copy link
Contributor Author

@jayudey-wf you want to do the honors?

@todbachman-wf
Copy link
Member

QA +1

  • Testing instruction
  • Dev +1's
  • Dev/QA +10 with detail of what was tested
  • Security review (if needed)
    • N/A
  • Unit tests created/updated
    • N/A
  • All unit tests pass
  • Confirm there is a ticket in the PR title
  • Rosie has run and reports properly the release the ticket will be included in
  • Rosie reports a clean dependency scan

Merging into master
@Workiva/release-management-pp

@rmconsole-wf
Copy link
Contributor

+1 from RM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants