Skip to content
This repository has been archived by the owner on May 17, 2019. It is now read-only.

Fix ownProps type signature #195

Merged
merged 2 commits into from
Mar 18, 2019
Merged

Conversation

akre54
Copy link
Contributor

@akre54 akre54 commented Mar 11, 2019

I incorrectly made ownProps a Maybe type in #190. It should be Object at the very least, or preferably pass along the actual Props type (see https://flow.org/en/docs/react/hoc/)

I also cleaned up the default argument for propName so that flow wouldn't complain.

rkaneriya
rkaneriya previously approved these changes Mar 12, 2019
@akre54
Copy link
Contributor Author

akre54 commented Mar 12, 2019

@KevinGrandon @ganemone ping

KevinGrandon
KevinGrandon previously approved these changes Mar 12, 2019
Copy link
Contributor

@KevinGrandon KevinGrandon left a comment

Choose a reason for hiding this comment

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

Seems fine to me, we should probably get a few flow fixtures in this app to ensure it works into the future. We can do that out of band though.

@KevinGrandon KevinGrandon dismissed stale reviews from rkaneriya and themself via 59b7601 March 12, 2019 19:19
@codecov
Copy link

codecov bot commented Mar 12, 2019

Codecov Report

Merging #195 into master will increase coverage by 3.09%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #195      +/-   ##
==========================================
+ Coverage   93.33%   96.42%   +3.09%     
==========================================
  Files           2        2              
  Lines          30       28       -2     
  Branches        5        5              
==========================================
- Hits           28       27       -1     
+ Partials        2        1       -1
Impacted Files Coverage Δ
src/hoc.js 94.73% <100%> (+4.26%) ⬆️

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 21fa4a0...59b7601. Read the comment docs.

@akre54
Copy link
Contributor Author

akre54 commented Mar 18, 2019

Bump @KevinGrandon @ganemone

@akre54
Copy link
Contributor Author

akre54 commented Mar 18, 2019

(agreed on the flow fixtures... kinda needs to be an e2e test for some of the underlying fusion-rpc stuff though)

@ganemone
Copy link
Contributor

!merge

@old-fusion-bot old-fusion-bot bot merged commit c5d84a1 into fusionjs:master Mar 18, 2019
@old-fusion-bot
Copy link

Triggered Fusion.js build verification: https://buildkite.com/uberopensource/fusion-release-verification/builds/1729

@rajeshsegu rajeshsegu mentioned this pull request Mar 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants