[SIP-6] Migrate visualizations to new directory structure. #5949
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kristw
force-pushed
the
kristw-vis-dir
branch
from
5698c28 to
843442a
Compare
kristw
force-pushed
the
kristw-vis-dir
branch
from
843442a to
3cd7a92
Compare
kristw
changed the title
kristw
changed the title
Codecov Report
@@ Coverage Diff @@
## master #5949 +/- ##
==========================================
- Coverage 63.54% 63.48% -0.07%
==========================================
Files 393 443 +50
Lines 23654 23753 +99
Branches 2638 2638
==========================================
+ Hits 15032 15080 +48
- Misses 8609 8660 +51
Partials 13 13
Continue to review full report at Codecov.
|
kristw
force-pushed
the
kristw-vis-dir
branch
2 times, most recently
from
b24be45 to
29859cd
Compare
conglei
approved these changes
Sep 27, 2018
| } = basicChartInput; | ||
| const { | ||
| dateFilter, | ||
| groupby, |
There was a problem hiding this comment.
nit. There are some inconsistencies on the name of groupby.In some chart, it is renamed as groupBy. it is better to make it consistent.
| colorScheme, | ||
| dateTimeFormat, | ||
| equalDateSize, | ||
| groupby: groupBy, |
There was a problem hiding this comment.
Here is one case that groupby is renamed.
There was a problem hiding this comment.
Good catch. Will fix this.
| [VIZ_TYPES.word_cloud]: () => | ||
| loadVis(import(/* webpackChunkName: "word_cloud" */ './wordcloud/WordCloud.js')), | ||
| loadVis(import(/* webpackChunkName: "word_cloud" */ './wordcloud/adaptor.jsx')), |
There was a problem hiding this comment.
Should we also rename this to WorldCloud/adaptor.jsx ?
There was a problem hiding this comment.
Git does not handle changing lower/upper case of same string well. I will leave this out of this PR to avoid unexpected git error.
kristw
force-pushed
the
kristw-vis-dir
branch
from
29859cd to
44d19eb
Compare
kristw
force-pushed
the
kristw-vis-dir
branch
from
44d19eb to
5245497
Compare
williaster
approved these changes
Sep 27, 2018
This was referenced Sep 27, 2018
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Oct 12, 2018
* Migrate Chord, Calendar * Migrate CountryMap * Add display name and rename Chord.jsx to Chord.js * migrate Histogram * add force-directed * migrate Heatmap * add horizon * migrate parallel coordinates * migrate partition * migrate pivot table * migrate rose * remove react-dom * migrate Sankey * migrate sunburst * migrate table * migrate treemap * migrate filterbox * migrate wordcloud * add paired t-test * fix unit test * remove renaming * rename fields
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
adaptor,transformPropsandReactXXXfor each componenttransformPropsfiles you can easily what happen toformDatabefore becoming usable by the vis components. Which one are transformed. Which one are just renamed. Which one are parsed (and start to question if this is the right place where parsing should happen)This PR is getting really big so I limited to the vis that are easy to convert (almost copy-paste) and leave the one that requires more work or needs more attention for follow-up PRs.
Included in this PR
Not included
@williaster @conglei