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

Add ChartClient (v2) #57

Merged
merged 7 commits into from
Dec 12, 2018
Merged

Add ChartClient (v2) #57

merged 7 commits into from
Dec 12, 2018

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Dec 11, 2018

🏆 Enhancements

Create ChartClient to facilitate calls to various APIs necessary for embeddable charts.

Refer to this design doc

Working on unit tests.

This PR replaces #44.

@kristw kristw requested a review from a team as a code owner December 11, 2018 01:14
@kristw kristw added the WIP Work in progress label Dec 11, 2018
@kristw kristw force-pushed the kristw--chart-client2 branch from 6791497 to 67c5377 Compare December 12, 2018 01:00
@kristw kristw added reviewable Ready for review #enhancement New feature or request and removed WIP Work in progress labels Dec 12, 2018
@codecov
Copy link

codecov bot commented Dec 12, 2018

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #57   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          65     66    +1     
  Lines         629    661   +32     
  Branches       70     77    +7     
=====================================
+ Hits          629    661   +32
Impacted Files Coverage Δ
packages/superset-ui-chart/src/query/FormData.ts 100% <ø> (ø) ⬆️
...ckages/superset-ui-chart/src/models/ChartPlugin.ts 100% <100%> (ø) ⬆️
...kages/superset-ui-connection/src/SupersetClient.ts 100% <100%> (ø) ⬆️
...kages/superset-ui-chart/src/clients/ChartClient.ts 100% <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 2896242...52a5849. Read the comment docs.

Copy link
Contributor

@xtinec xtinec left a comment

Choose a reason for hiding this comment

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

Woot! Tests!
A few changes needed to get the demo working end to end.

@kristw kristw force-pushed the kristw--chart-client2 branch from 9e0a4ec to 1f346c2 Compare December 12, 2018 02:36
type BaseFormData = {
datasource: string;
// eslint-disable-next-line camelcase
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] We could disable the camelcase rule for the entire BaseFormData block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call.

@kristw kristw merged commit d8d2bb2 into master Dec 12, 2018
@delete-merged-branch delete-merged-branch bot deleted the kristw--chart-client2 branch December 12, 2018 21:18
@kristw kristw added this to the v0.8.0 milestone Feb 1, 2019
kristw added a commit that referenced this pull request Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
#enhancement New feature or request reviewable Ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants