-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
refactor: Migrates legacy Sunburst charts to ECharts and removes legacy code #26350
refactor: Migrates legacy Sunburst charts to ECharts and removes legacy code #26350
Conversation
f71c6ff
to
0d01d57
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #26350 +/- ##
==========================================
+ Coverage 69.06% 69.24% +0.18%
==========================================
Files 1930 1924 -6
Lines 75275 75030 -245
Branches 8429 8427 -2
==========================================
- Hits 51985 51954 -31
+ Misses 21143 20929 -214
Partials 2147 2147
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@michael-s-molina I was wondering if the title of the PR "refactor: Removes the legacy Sunburst chart" should be updated? A non-informed reader may recoil thinking that you're simply removing the legacy chart, whereas in actuality you're actually migrating all their legacy charts to the ECharts equivalent and annexing all the legacy code. |
Good callout. I changed the PR title to "refactor: Migrates legacy Sunburst charts to ECharts and removes legacy code" |
Is Sunburst v2 going to be renamed to "Sunburst" in the viz picker since there will be only one? |
b2c4e65
to
9c6ea29
Compare
@yousoph Good call. Renamed. |
SUMMARY
As part of the 4.0 approved initiatives, this PR removes the legacy Sunburst chart by:
TESTING INSTRUCTIONS
1 - Make sure all legacy Sunburst charts were converted to Sunburst v2
2 - Make sure legacy Sunburst is not available anymore in the viz picker
ADDITIONAL INFORMATION