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

RFC Move config code into silverstripe/config #10728

Open
GuySartorelli opened this issue Mar 17, 2023 · 2 comments
Open

RFC Move config code into silverstripe/config #10728

GuySartorelli opened this issue Mar 17, 2023 · 2 comments

Comments

@GuySartorelli
Copy link
Member

GuySartorelli commented Mar 17, 2023

Motivation

Struck out as it seems it was distracting from the point.

I've recently been working on a set of tools written using symfony/console with the intention of lowering the bar for entry for new Silverstripe developers by providing a dockerised apache-based local development environment.

The more I work on this, the clearer it becomes that it won't be able to (and shouldn't try to) cater to everyone's needs out of the box - but a plugin system could allow people to add on any features or infrastructure they might need.

I already intend to use silverstripe/config in this plugin system, but there are some parts to Silverstripe framework's Configuration API which I'd have to essentially rewrite at this stage because they're in silverstripe/framework instead of silverstripe/config.

Concept

The following classes/traits in silverstripe/framework have no coupling to other code in framework, and could be easily moved into silverstripe/config:

This collection of code effectively makes up the configuration API in Silverstripe framework. Without these, interacting with silverstripe/config is not as familiar and is somewhat more cumbersome, and that makes silverstripe/config on its own less useful than it could be.

Namespaces

Because this proposal comes so late after the CMS 5 beta release, I suggest we don't change the namespaces for now - instead we lift-and-shift the relevent code into silverstripe/config, and remove the code from framework. silverstripe/config is already a dependency of framework.

Note that with this approach we could do this during a minor release if we wanted to (the API wouldn't actually change) - but I propose we try and get it done before the CMS 5 stable release.

We can then change the namespaces when we start working on the CMS 6 major release cycle.

Bonus Points

SilverStripe\Core\Config\CoreConfigFactory provides some familiar configuration rules such as classexists, extensionloaded, envvarset, etc. It would be very beneficial to have some of this defined in silverstripe/config - but for right now that class is too tightly coupled to silverstripe/framework to just lift-and-shift it.
I could see that being considered for CMS 6, but it is out of scope for this RFC.

Benefits:

  • Projects can benefit from the full Configuration API without needing the entire framework
  • Smaller decoupled components are easier to maintain
    • This reduces the risk we'll end up coupling those classes with framework in the future

Drawbacks:

  • We'll probably want to change the namespaces for these classes/traits at some stage, which will introduce some (slight) upgrade pains.
    • Mitigated by being easy to do a find-and-replace
@michalkleiner
Copy link
Contributor

A bit of devil's advocate here, but does the tooling need to depend on silverstripe/config? It's not the industry standard for managing infrastructure or containers, and it could perhaps create more friction and lower portability.
I'd support some more tooling around dev env setup, but that could be a completely separate module with no bearing on silverstripe internals or configs whatsoever.
If it's about moving the config out of the framework to the config module, that's fine, and maybe the motivation doesn't need to mention the dev env bit at all as it might be a red herring.

@GuySartorelli
Copy link
Member Author

The motivation is more "this is what got me thinking about it" and provides one possible use case for using Silverstripe/config outside of the context of Silverstripe/framework. I'll strike it out it as it sounds like it's distracting from the point.

@GuySartorelli GuySartorelli changed the title RFC: Move config code into silverstripe/config RFC Move config code into silverstripe/config Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants