Skip to content

Allow for selectively INCLUDING config types instead of EXCLUDING via config/plugins.ts #96

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

Open
EsGeh opened this issue Aug 2, 2023 · 9 comments
Labels
good first issue Good for newcomers

Comments

@EsGeh
Copy link

EsGeh commented Aug 2, 2023

Feature request

Summary

Quick summary what's this feature request about.
Allow for selectively including config types via config/plugins.ts.

Why is it needed?

I'd like to be able to only include only very few very specific types to be synchronized, e.g. core-store.plugin_content_manager_configuration_content_types::api::....
With the current config format this is almost impossible, because there is no way to specify what to include, but only what to exclude from the defaults. So in order to include very specific config types, one would have to list "everything else" in the excludedConfig field of the config, which is unpractical, and in many cases impossible.

Suggested solution(s)

  1. Conservative solution:
    Consider entries in customTypes even if they match some pattern listed in excludedConfig. This way you may exclude "everything" by listing all default types in excludedConfig and then explicitly list what you'd like to include via customTypes.

  2. More flexible solution

  • By default: no config types are considered.
  • Redesign plugin config format: Only what is listed inside a field named includedConfig is considered

Related issue(s)/PR(s)

Unknown.

@boazpoolman boazpoolman added enhancement New feature or request Needs investigation Further information is requested labels Aug 2, 2023
@boazpoolman
Copy link
Member

Hi @EsGeh,

Thanks for reporting this issue.
I agree with you that having a way to include specific configs would be beneficial for the plugin.

I would suggest going with solution nr 2.
We would introduce a new config key called include which works alongside of the allready existing excludedConfig and excludedTypes. The config should work the same as the excludedConfig. Having an array of strings which can be just one specific config, or just the start of it to include multiple configs at once. That can be done with the native javascript startsWith function to match the start of a string.

If a specific config is for some reason excluded and included through the different config options, then the exclude options should have priority. So a config that is included and excluded will not be synced with the plugin.

Also this new include config array should be able to be empty. That way everything will be included, just like it works now. That way we preserve backwards compatibility.

@boazpoolman boazpoolman added good first issue Good for newcomers and removed Needs investigation Further information is requested labels Sep 11, 2023
@cekpowell
Copy link

@boazpoolman - Would love to see this get implemented for something im working on.

Happy to raise a PR with the changes based on your description above. Should only be small as its just copying & inverting the excludedConfig field.

Let me know.

@boazpoolman
Copy link
Member

That would be great!

Please check out the CONTRIBUTING.md to learn how to setup the development environment.

@cekpowell
Copy link

Thanks - will take a look at this today!

@cekpowell
Copy link

@boazpoolman I have completed this work now but cannot submit my changes - I think i need to be added as a maintain to the project? Could you do this so I could submit my PR ?

@boazpoolman
Copy link
Member

Hi @cekpowell,

That is great news.

You should be able to fork the repo, commit and push your changes to it, and then create a PR on the original repo, using your fork as the source branch.

@cekpowell
Copy link

Perfect will do thanks!

@cekpowell
Copy link

@boazpoolman PR up and ready!

#168

Thanks!

@cekpowell
Copy link

@boazpoolman - Actioned those comments - thanks!

@boazpoolman boazpoolman removed the enhancement New feature or request label Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants