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

UIP-3041 Add a null check when canceling the store stream subscription #138

Merged
merged 1 commit into from
Feb 15, 2018

Conversation

jacehensley-wf
Copy link
Contributor

Ultimate problem:

I ran into an issue when implementing a shouldComponentUpdate method in a class that extends BuiltReduxUiComponent.

How it was fixed:

  • Null check _storeSubs

Testing suggestions:

  • CI still passes

Potential areas of regression:

N/A


FYA: @greglittlefield-wf @aaronlademann-wf @jacehensley-wf @clairesarsam-wf

@jacehensley-wf jacehensley-wf requested a review from a team as a code owner February 15, 2018 18:17
@aviary2-wf
Copy link

Raven

Number of Findings: 0

Magpie

Number of findings: 0

Copy link
Contributor

@aaronlademann-wf aaronlademann-wf left a comment

Choose a reason for hiding this comment

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

+1

@rmconsole2-wf rmconsole2-wf changed the title Add a null check when canceling the store stream subscription UIP-3041 Add a null check when canceling the store stream subscription Feb 15, 2018
@brianbolton-wf
Copy link

+1

1 similar comment
@kaitlynchilders-wk
Copy link

+1

@aaronlademann-wf
Copy link
Contributor

QA +1

  • Testing instruction
  • Dev +1's
  • All unit tests pass
  • Rosie ran/Rosie comment displays expected info
  • Dependency Scan Clean

Merging.

@codecov-io
Copy link

codecov-io commented Feb 15, 2018

Codecov Report

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

@@           Coverage Diff           @@
##           master     #138   +/-   ##
=======================================
  Coverage   94.54%   94.54%           
=======================================
  Files          33       33           
  Lines        1591     1591           
=======================================
  Hits         1504     1504           
  Misses         87       87

@aaronlademann-wf aaronlademann-wf merged commit 48152b4 into Workiva:master Feb 15, 2018
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