superset
superset copied to clipboard
chore: Move charts to src/pages folder
SUMMARY
Moved ChartCreation & ChartList to src/pages
folder to apply direction outlined in #13632
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in SIP-59)
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
Codecov Report
Merging #22230 (603c8f0) into master (08f45ef) will decrease coverage by
0.03%
. The diff coverage is33.33%
.
@@ Coverage Diff @@
## master #22230 +/- ##
==========================================
- Coverage 65.61% 65.57% -0.04%
==========================================
Files 1869 1869
Lines 71582 71549 -33
Branches 7814 7821 +7
==========================================
- Hits 46968 46918 -50
- Misses 22573 22603 +30
+ Partials 2041 2028 -13
Flag | Coverage Δ | |
---|---|---|
javascript | 53.90% <33.33%> (-0.09%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
...uperset-frontend/src/pages/ChartCreation/index.tsx | 61.11% <ø> (ø) |
|
...uperset-frontend/src/pages/ChartList/ChartCard.tsx | 52.17% <ø> (ø) |
|
superset-frontend/src/pages/ChartList/index.tsx | 54.61% <ø> (ø) |
|
...set-frontend/src/views/CRUD/welcome/ChartTable.tsx | 62.50% <ø> (ø) |
|
superset-frontend/src/views/routes.tsx | 55.00% <33.33%> (ø) |
|
superset-frontend/src/setup/setupFormatters.ts | 0.00% <0.00%> (-100.00%) |
:arrow_down: |
superset-frontend/src/setup/setupClient.ts | 0.00% <0.00%> (-70.00%) |
:arrow_down: |
superset-frontend/src/preamble.ts | 0.00% <0.00%> (-44.00%) |
:arrow_down: |
...rontend/src/components/DeprecatedSelect/styles.tsx | 52.83% <0.00%> (-28.31%) |
:arrow_down: |
...erset-frontend/src/profile/components/Security.tsx | 75.00% <0.00%> (-25.00%) |
:arrow_down: |
... and 74 more |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
I think it looks good, but pinging @michael-s-molina for review as his more familiar with the proposed folder structure.
1 concern though - if we're moving stuff to the new /pages
folder, shouldn't we move all pages at once, so that there's no inconsistency before the rest of the pages are moved?
@kgabryje If we move all pages at once we break a lot of open PR. I guess the best strategy would be to move them one by one and select only those pages that have little activity so as not to break current PRs.
@kgabryje If we move all pages at once we break a lot of open PR. I guess the best strategy would be to move them one by one and select only those pages that have little activity so as not to break current PRs.
I agree. I'm taking the iterative approach to reduce impact.
@EugeneTorap One thing I was planning to do before start moving components to the pages folder was to create a sketch of how it would look in the end. This would help with folder naming definitions, how to break the work into multiple PRs, and with the reviews. This is also a required step for the final wiki page that will contain the final proposed structure. The idea would be to do a before/after of the components involved with the pages so we can validate the structure. Something like:
Before
src
addSlice
views
CRUD
chart
After
src/pages
charts
chartCreation
chartList
I think the best place for this work is a Github discussion with the title SIP-61 src/pages structure proposal
. Given that you're interested in the project, would you like to draw this sketch and open the discussion?
@michael-s-molina Deal me in!
@michael-s-molina Deal me in!
Cool. I'll wait for the discussion before merging this PR then. Feel free to ping me when the discussion is open.
@michael-s-molina Done! #22330
@michael-s-molina Can we merge it?
/testenv up
@michael-s-molina Ephemeral environment spinning up at http://35.91.199.214:8080. Credentials are admin
/admin
. Please allow several minutes for bootstrapping and startup.
Ephemeral environment shutdown and build artifacts deleted.