MatanBobi commented on issue #13013:
URL: https://github.com/apache/superset/issues/13013#issuecomment-788778951


   > 
   > * SIP-57, in favor of Semantic Versioning, is still in the VOTE process. 
This should be considered here. The current NPM publishing scripts bump ALL 
touched packages in parallel. We may need to pay more attention to semantically 
bumping the version numbers of _individual_ plugins, or establishing some 
process to make sure that ALL plugins are bumped to the highest/safest level 
(i.e. a breaking change on one package means they all get a major number bump).
   > * I really prefer the idea of using the `src` path for packages, rather 
than having to jump through the hoops of publishing/bumping packages internally 
in Superset. Removal of this overhead is the main motivation for the 
homecoming, in my opinion. We should indeed address the cadence of when we 
publish new plugin versions. My off-the-cuff suggestion would be to make it 
part of the Superset OSS release procedure, so they're in tighter lock-step. We 
_could_ even consider aligning version numbers between Superset and the 
internal packages.
   > * Regarding the viz plugins, we would still need to keep some of them 
external, on `superset-ui`. One motivation to externalize them was around the 
licensing of dependencies being incompatible with Apache. I believe this is 
likely to remain the case, so only the "safe" ones can come home. We'll need to 
support `npm link`, Storybook, release actions, etc on `superset-ui` to support 
those remaining packages.
   
   Thanks everyone for all the helpful inputs!
   
   @rusackas 
   1. IMO we should use lerna independent versioning, I don't think that a 
breaking change in one plugin should cause a major version bump in another 
version.
   2. If we decide to align the version numbers we'll get a problem with 
semantic versioning and major version bumps you raised on the first point 
wouldn't we?
   3. That's a good point. Is there a list of the plugins/packages we can move 
based on the license? Since I couldn't find any licensing on the plugins, just 
on the whole `superset-ui` repo.
   
   > I think it would be nice if the `superset-ui` npm packages would be synced 
with the official release versions; when a new Superset version is released, 
the all `superset-ui` packages would be released with the same version on npm. 
If we stay true to semantic versioning (the SIP has now passed), people would 
know that if their external applications is using e.g. v. 1.1.0 of 
`superset-ui`, it will be compatible with a backend running 1.5.0.
   
   @villebro I do agree that it can be nice, but I'm not sure it is really 
helpful. This means that whenever you want to release a patch to a plugin 
you'll also bump all the packages and the superset-ui core module, sounds 
redundant to me but I'm not familiar with the whole release mechanism so I 
might be missing some stuff.
   
   
   @kristw thanks for the helpful inputs!
   
   > **History-1:** The quality of the code in `apache/(incubator-)superset` 
was not great. Some parts were well tested. Some not. Lints were enabled only 
for certain parts. `superset-ui` was created to be stricter: requiring 100% 
test coverage and TypeScript. We rewrote many core parts including massive 
clean up of all charts, then replaced legacy code in the main repo with these 
newer components.
   > 
   > **Current:** 🟢 Main repo has more TypeScript + lints and overall code 
quality has improved.
   
   
   I believe the issue we have now is the opposite one.. `superset-ui` doesn't 
have tests almost at all. The coverage configs are all on 0.
   
   > History-3: Better tooling and flexibility. We experimented and adopted 
bots, conventional commits, PR previews (Netlify then later Zeit/Vercel), 
semi-automatic npm packages release via github actions. These are difficult to 
experiment and configure on repos under apache org which we don't have admin 
rights.
   > 
   >  Current: 🔴 We still don't have admin rights on apache/superset and some 
apps are not allowed.
   
   That's an important one, the question is, is that a blocker?
   
   > History-5: Some legacy chart plugins were added by community and caused 
maintenance problems for committers over time. The plugins system and 
publishing the packages on npm hoped to decentralize this a bit. Each 
org/person can have repo of their custom plugins with repo structure forked 
from superset-ui-plugins.
   > 
   >  Current: 🟡 Can still have boilerplate of standalone plugin repo for 3rd 
party who wants to create plugin, but it will still have issues: keep build 
config in sync and npm link. If the main developers are no longer dogfooding 
this approach, it will likely be broken after a while.
   > 
   
   I think we should still maintain that approach, having developers create 
PR's on the main repo might be a serious bottleneck.
   As long as there are still plugins left outside on `superset-ui` due to 
licensing I guess we'll keep maintaining it, but even after, we need to 
maintain a doc page explaining how to do it and also not to break that 
mechanism.
   
   Regrading the open issues:
   - Repo tools: PR preview Storybook via Zeit and Chromatic UI - Who can check 
with Apache admins what are we allowed to do?
   - Reduced build time from smaller codebase and not having to share the 
GitHub Action runner pool with other Apache projects - I'll work on this one as 
part of this SIP.
   - Revise automatic NPM release - We need to talk about this one since IMO we 
need to define a way to publish plugins which is unrelated to the main repo. 
Also, since the FE will be a part of the lerna monorepo, we should know that a 
new version will be released once it has changes (though we might not rebuild 
the docker image of-course).
   - Figure out npm packages release cadence: How often will the plugins be 
published? - IMO this should be automatic even if we decide to use the `src` 
folder instead of the npm package. Every change will be released using lerna 
and github actions.
   - Maintain quality bar e.g. 100% test coverage guarantee for core packages - 
As I stated before, the current status of `superset-ui` isn't as good as we 
thought it is..
   - 3rd party chart plugins: How would someone develop chart plugins that are 
only useful for his/her org (without dropping code in apache/superset)? - This 
doesn't sound like something which should be coupled to this SIP but I do agree 
that we need to think about it. At the moment, one already needs to edit code 
in superset to add plugins. We might need to figure out a way to extend the 
plugins without editing any code.
   
   Thanks everyone for the valuable inputs! I'm starting to work on this one 
but I think we should setup some milestones we want to see. I'll try to update 
the PR and SIP with specific milestones.


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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to