[GitHub] [superset] simcha90 commented on pull request #17208: feat: Dynamic dashboard component

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

simcha90 commented on pull request #17208:
URL: https://github.com/apache/superset/pull/17208#issuecomment-1014541656


   > First pass comments. Code looks great, but my main question is shouldn't we move the central building blocks to `@superset-ui/core`, like the central types/interfaces and registries? If we'd do that, we could
   > 
   > 1. Develop the plugins independently from the Superset codebase.
   > 2. Add a dashboard component template to the Yeoman generator. See #18027 which was merged just recently and made the system work again after the monorepo migration
   > 3. Publish dashboard component plugins to npm
   > 4. Pin plugins to specific versions etc
   > 
   > Etc. Also, I think we should add tests as early as possible, which are missing here. By moving the relevant parts to `@superset-ui/core` we would automatically have 100 % test coverage requirements, but in addition, we should also have at least a few RTL tests that make sure the relevant Dashboard flows work (viewing a dashboard with a plugin + adding and removing a component)
   
   May be functional registry can be moved to `superset-core`, but meantime I don't see some special need for it because:
   1. we can do it also now, I think this PR enough for our company needs to start develop our plugins
   2. I'm not sure that we have capacity for it, @amitmiran137 do we have?
   3, 4 Can be done also now, we don't need move code to `superset-core` for these things
   
   P.S. other stuff fixed


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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