[GitHub] [incubator-superset] ktmud commented on issue #9187: [SIP-38] Visualization plugin refactoring

[GitBox <gi...@apache.org>]

ktmud commented 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 doubts for me.
   
   Just to be clear, what I had in mind when commenting [#8638](https://github.com/apache/incubator-superset/pull/8638) was to **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 proposed in [SIP-34](https://github.com/apache/incubator-superset/issues/8976) finally starts to taking shape... 
   
   It might take a lot of back and forth 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_ seems far overweight the potential benefits it may bring in the future.
   
   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 so. But for core visualizations, I'm not so sure. Superset as a software needs to ship with some visualizations anyway and having the source code in one place makes it easier for new developers to start writing their own plugins---and for existing contributors to upgrade.
   
   A true plugin ecosystem is achievable even with the frontend registry code and backend API placed in the same repo.
   
   ----
   
   @kristw , to mitigate some of the concerns raised during the initial separation of repos, 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.
   
   ----
   
   Another alternative is to have a frontend monorepo within `incubator-superset`, publishing `superset-ui` and "official" plugins as separate packages but track then under the same git repo as the backend code. This way embedadable Superset charts is also still possible.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org