Skip to content
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

Fixes for extension config loading (issue #847) #884

Closed
wants to merge 1 commit into from

Conversation

mblythe86
Copy link
Contributor

This seems to fix the problems of generating a fresh config with extension settings, saving extension config changes to the file, and loading extension settings from a config file that already includes them.

As noted in #847, this isn't a complete fix. There's still the problem of adding an extension to an existing install. I don't think that can be fixed in pigallery2. I think that needs to be fixed in typeconfig.

* Actually add/remove extensions from `Config.Extensions.extensions`
* Fix the file path where `ExtensionConfigTemplateLoader` looks for extensions
* Prevent the first `loadSync()` call from creating a `config.json` file
  Works around bpatrik/typeconfig#27
@@ -79,6 +80,14 @@ export class ExtensionManager implements IObjectManager {
);
extList.sort();

// delete not existing extensions
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wanted to avoid this. (i.e.: I removed this laoding here on porpuse.)

I would like to have a single place that is responsible loading extension config (even if they do not exists, then its an empty config).
That is why I moved all code to the extension config loader.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a closer look at the extension config loader. I didn't see anywhere else that Config.Extensions.extensions.push(new ServerExtensionsEntryConfig(ep))); happened, so the extension configs never actually got loaded into Config.

@bpatrik
Copy link
Owner

bpatrik commented Apr 13, 2024

I will reject this as fixes for the same issue should be already part of the master

@bpatrik
Copy link
Owner

bpatrik commented Apr 29, 2024

I will reject it as I think the issue that this PR try to solve is already fixed.
Sorry and thank you for the effort. I appreciate it!

@bpatrik bpatrik closed this Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants