-
Notifications
You must be signed in to change notification settings - Fork 378
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
fix: fix loading of extensions by removing sample-ui-plugin #4251
Conversation
@@ -245,12 +245,15 @@ class ExtensionManager { | |||
const extensionInstallPath = path.dirname(fullPath); | |||
const packageJson = (await readJson(fullPath)) as PackageJSON; | |||
const isEnabled = packageJson?.composer && packageJson.composer.enabled !== false; | |||
const metadata = getExtensionMetadata(extensionInstallPath, packageJson); |
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.
why was this moved out of the if
statement if the else if
does not contain any reference to metadata
?
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.
Good question. I had a version where I was just disabling the extension but setting the other metadata fields. I felt like removing the extension is better but forgot to move this back.
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.
we can just move it back on some other PR
* main: fix: update l10n file (microsoft#4247) fix: fix loading of extensions by removing sample-ui-plugin (microsoft#4251) fix: split qna resource to another template (microsoft#4212) feat: UI Schema - Recognizer (microsoft#4135) fix: Change http to https for petstore.swagger.io calls (microsoft#4238) feat: install remote extensions from npm (microsoft#4224) fix: refactored select skill ui-plugin (microsoft#4207) feat: Added fieldSets to UIOptions (microsoft#4231) fix: New LG template not sync to other locale files (microsoft#4230)
* main: fix: update l10n file (microsoft#4247) fix: fix loading of extensions by removing sample-ui-plugin (microsoft#4251) fix: split qna resource to another template (microsoft#4212) feat: UI Schema - Recognizer (microsoft#4135) fix: Change http to https for petstore.swagger.io calls (microsoft#4238) feat: install remote extensions from npm (microsoft#4224) fix: refactored select skill ui-plugin (microsoft#4207) feat: Added fieldSets to UIOptions (microsoft#4231) fix: New LG template not sync to other locale files (microsoft#4230)
* main: fix: update l10n file (#4247) fix: fix loading of extensions by removing sample-ui-plugin (#4251) fix: split qna resource to another template (#4212) feat: UI Schema - Recognizer (#4135) fix: Change http to https for petstore.swagger.io calls (#4238) feat: install remote extensions from npm (#4224) fix: refactored select skill ui-plugin (#4207) feat: Added fieldSets to UIOptions (#4231) fix: New LG template not sync to other locale files (#4230)
…t#4251) * fix loading of extensions by removing sample-ui-plugin * remove builtin extension from manifest if not enabled
Description
In #4224, the way builtin plugins are loaded changed which breaks when the
extensions.json
file already existed, due to a failure to load thesample-ui-plugin
. This just removes that from the manifest.Task Item
#minor