Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

feat: deprecate old SuperChart API that accepts chartProps #202

Merged
merged 1 commit into from
Aug 14, 2019

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Aug 13, 2019

馃挃 Breaking Changes

  • No longer accept chartProps as a single prop in <SuperChart>
  • Must specify each field in chartProps individually. e.g.
<SuperChart width={} height={} formData={} />

instead of

<SuperChart chartProps={{width: ... , height: ...,  formData: ...}} />

@kristw kristw requested a review from a team as a code owner August 13, 2019 18:47
@kristw kristw added reviewable Ready for review and removed reviewable Ready for review labels Aug 13, 2019
@codecov
Copy link

codecov bot commented Aug 13, 2019

Codecov Report

Merging #202 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #202      +/-   ##
==========================================
- Coverage   99.91%   99.91%   -0.01%     
==========================================
  Files         102      101       -1     
  Lines        1221     1215       -6     
  Branches      293      292       -1     
==========================================
- Hits         1220     1214       -6     
  Partials        1        1

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 f8d7042...9e6abba. Read the comment docs.

@netlify
Copy link

netlify bot commented Aug 13, 2019

Deploy preview for superset-ui ready!

Built with commit 9e6abba

https://deploy-preview-202--superset-ui.netlify.com

@kristw
Copy link
Contributor Author

kristw commented Aug 13, 2019

Please ignore codecov warning. The reduced percentage (<0.01%) is due to code deletion.

Copy link

@graceguo-supercat graceguo-supercat left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@williaster williaster left a comment

Choose a reason for hiding this comment

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

I like this approach, it's less opaque 馃憤

@kristw kristw merged commit af877c6 into master Aug 14, 2019
@delete-merged-branch delete-merged-branch bot deleted the kristw--simplify-super-chart branch August 14, 2019 17:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
reviewable Ready for review size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants