Skip to content
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

chore(generator-superset): migrate to monorepo #17829

Merged
merged 4 commits into from
Dec 21, 2021

Conversation

villebro
Copy link
Member

@villebro villebro commented Dec 20, 2021

SUMMARY

Currently the generator-superset Yeoman generator template generates plugins that require the old superset-ui monorepo. This is a first-pass at fixing the generator by changing the following:

  • Makes the plugin standalone by adding scripts for building the package, along with necessary babel and typescript configs
  • Removes the option to create a superset-ui package - now the generator only supports creating plugins
  • Remove the option to create class based components - now only functional components are provided out of the box
  • Removes the option to add badges to the README file

SCREENSHOT

This plugin has been generated with the updated template:
image

TESTING INSTRUCTIONS

  1. Go into the superset-frontend/packages/generator-superset folder and run npm i
  2. Go into ~/src and run mkdir superset-plugin-chart-hello-world
  3. Run yo @superset-ui/superset and answer all questions
  4. Run npm i --force && npm run build && npm run test && npm pack and see that the new package works and can be packaged for npm
  5. Add the plugin to the Superset MainPreset.js and see it work

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
Copy link

codecov bot commented Dec 21, 2021

Codecov Report

Merging #17829 (8999828) into master (b82da5c) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17829      +/-   ##
==========================================
- Coverage   67.17%   67.15%   -0.02%     
==========================================
  Files        1609     1608       -1     
  Lines       64796    64801       +5     
  Branches     6855     6851       -4     
==========================================
- Hits        43528    43519       -9     
- Misses      19412    19423      +11     
- Partials     1856     1859       +3     
Flag Coverage Δ
javascript 53.87% <ø> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...-frontend/src/views/CRUD/welcome/ActivityTable.tsx 54.66% <0.00%> (-9.81%) ⬇️
superset-frontend/src/setup/setupColors.ts 81.81% <0.00%> (-8.19%) ⬇️
superset-frontend/src/utils/localStorageHelpers.ts 84.00% <0.00%> (-4.89%) ⬇️
...perset-frontend/src/views/CRUD/welcome/Welcome.tsx 76.06% <0.00%> (-2.75%) ⬇️
...ntend/src/explore/components/ExploreChartPanel.jsx 14.66% <0.00%> (-1.13%) ⬇️
...frontend/src/SqlLab/components/SqlEditor/index.jsx 51.76% <0.00%> (-0.62%) ⬇️
...frontend/src/views/CRUD/welcome/DashboardTable.tsx 61.44% <0.00%> (-0.46%) ⬇️
...set-frontend/src/views/CRUD/welcome/ChartTable.tsx 69.86% <0.00%> (-0.41%) ⬇️
...nd/src/explore/components/DataTablesPane/index.tsx 76.69% <0.00%> (-0.23%) ⬇️
...perset-frontend/src/views/CRUD/chart/ChartList.tsx 62.14% <0.00%> (ø)
... and 5 more

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 b82da5c...8999828. Read the comment docs.

Copy link
Member

@zhaoyongjie zhaoyongjie left a comment

Choose a reason for hiding this comment

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

It's works in my local environment.
image

@@ -0,0 +1,20 @@
const { getConfig } = require('@airbnb/config-babel');
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to use babel config without airbnb/config-babel?

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems to bring in a bunch of good defaults (would otherwise have to do lots of other configs here), so I think it's a good idea to use it.

Copy link
Member

@zhaoyongjie zhaoyongjie left a comment

Choose a reason for hiding this comment

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

LGTM!

@villebro villebro merged commit 19daf65 into apache:master Dec 21, 2021
@villebro villebro deleted the villebro/generator-superset branch December 21, 2021 09:44
shcoderAlex pushed a commit to casual-precision/superset that referenced this pull request Feb 7, 2022
* chore(generator-superset): migrate to monorepo

* add todo and remove webpack reference from template

* fix linting errors

* remove redundant test file
bwang221 pushed a commit to casual-precision/superset that referenced this pull request Feb 10, 2022
* chore(generator-superset): migrate to monorepo

* add todo and remove webpack reference from template

* fix linting errors

* remove redundant test file
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels preset-io size/L 🚢 1.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants