-
Notifications
You must be signed in to change notification settings - Fork 7
Release Branch for 5.4.0 #357
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
Conversation
…gins to test that
|
@jvigliotta , if this doesn't close #317 , we shouldn't say it does |
…move special handling for installing summary widgets
|
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did leave a comment re: if plugins are enabled or not. But looks good.
| const pluginExists = openmct.plugins[plugin] || openmct.plugins.example[plugin]; | ||
| const examplePluginExists = openmct.plugins.example[plugin]; | ||
| const pluginExists = openmct.plugins[plugin] || examplePluginExists; | ||
| const pluginEnabled = pluginConfig?.enabled; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we follow the previous format summary widgets followed, or just require an enabled flag now?
config.plugins.summaryWidgets === true ||
config.plugins.summaryWidgets?.enabled === true
closes #348
closes #208
closes #336
closes #209
This is a larger merge of the following items (in addition to various fixes for parity with latest Open MCT):