ktmud edited a comment on issue #9187: [SIP-38] Visualization plugin refactoring URL: https://github.com/apache/incubator-superset/issues/9187#issuecomment-589912159 Thanks for the writeup, @rusackas! This raises a lot of great points and cleared many of my confusions. I’m still new to Superset, so apologies if I missed some historical context. To expand on what I said in [#8638](https://github.com/apache/incubator-superset/pull/8638) was should we **temporarily** move plugins and their registries back to `incubator-superset` until things are more settled——plugin API matured, core plugin quality under control, and [the big UI overhaul (SIP-34)](https://github.com/apache/incubator-superset/issues/8976) finally starting to take shape. My biggest concern was it might take a lot of efforts to get there and having to sync code between multiple repos slows the whole process down. Even if we can solve the linking issue with a custom `npm` command, people still need to make (and review) two (maybe three) PRs whenever the registry or control panel API needs to change. Overall, the overhead the 3-repo structure imposes _right now_ seemed overweight the potential benefits it may bring in the future—considering there might be other solutions to achieve more or less the same goal. In general, I do agree it makes a lot of sense to have optional visualizations as separate packages and in their own repo. Future Superset admins may even install a plugin from the web UI so it's only natural to do this. But for core visualizations, I had more doubts. Superset as a software needs to ship with some visualizations anyway and having the source code in one place makes it easier for developers to start writing their own plugins or upgrading an existing one——simpler dev env setup, easier navigation, better git history... A true plugin ecosystem IS achievable even with the frontend registry code and backend API placed under the same repo. ---- @kristw , to mitigate some of the original concerns, maybe we can take following actions (if we haven't): - Officially freeze the `visualization` folder, no PRs will be accepted or reviewed until we clean things up and plugin API stabilized - Require all new visualizations to have unit test that do not depend on Superset backend API - Place additional more strict `.eslintrc` to the folders of migrated components. ---- Alternatively, we may also have a frontend "monorepo" within `incubator-superset`, publishing `superset-ui` and "official" plugins as separate packages but track them under the same git repo with the backend code. This way embeddable charts also get to stay on the roadmap.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
