silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
722 stars 821 forks source link

RFC Move config code into silverstripe/config #10728

Open GuySartorelli opened 1 year ago

GuySartorelli commented 1 year ago

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:

Drawbacks:

michalkleiner commented 1 year ago

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 commented 1 year ago

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.