Config Object

[l0gicgate]

I think that it's probably easier to start with this base versus rewriting #13 entirely.

We have quite a few decisions to make at this particular moment in regards to this:

• Validation seems clunky, I need input to see if there's an easier way to do this.
• Default values / Required Values
• Do we give the option of setting parameters on the fly on the configuration object? Is this necessary? In which case would you need to change parameters on the config at runtime. I almost feel like the setters are unnecessary
• Same goes for the AbstractCommand object, setting the Application's configuration object from within a command seems like an anti-pattern, so I did not include it. I did include a getter though because I think it could be needed.
• Error message formats. We need to nail those down
• We need to define what can be extensible in user land. I feel like Application and Configuration should be extensible. The use case I imagine is a highly customizable console which just blends in with your project versus being standalone for Slim. This goes with the idea that we are allowing users to include external commands. I made the APPLICATION_NAME and VERSION constants protected so one could extend them easily to override them. Let me know what y'all think

Note: This is mostly untested code and it's just so we can nail down the initial objects. Once we are in agreement I will write the tests.

Progress:

• [x] Initial Implementation
• [x] Make decisions
• [x] Write tests

Closes #6

[adriansuter]

I would move the call to validation into the Config constructor? Otherwise someone could construct a Config with invalid params.

[l0gicgate]

@adriansuter a way to mitigate that would be to make the constructor protected, what do you think? Also, could you show an example of an invalid configuration? Since all the constructor parameters are typed. I suppose you could do a white space string but that would be pretty dumb.

[adriansuter]

Yes, but we should avoid those dumb (sometimes unintended) errors, shouldn't we?

The setters can be removed, in my opinion. I don't see an example usage - and if needed, they could be added later on.

I have a naming question. Is it Configuration or Config? After all, it is Application and not App.

[l0gicgate]

@adriansuter right. I think that maybe we should rename Application to App instead that would be more cohesive with the main Slim repo right?

I’ll remove the setters and make the constructor protected!

[ABGEO]

* Validation seems clunky, I need input to see if there's an easier way to do this.

What do you think about Symfony Validator? Using this component we can easily validate the config object.

* I almost feel like the setters are unnecessary

I agreed. I think setters and unsetters are not needed. I have no idea when we would like to change the configuration at runtime.

[flangofas]

@l0gicgate One more question to add to the decision list.

1. Where is the executable slim going to be placed: a. In the usual vendor/bin/slim b. In the project's root directory c. installed globally as @ABGEO07 suggested in #13

   a makes more sense to me because then config directory can be resolved and used also as project's root directory which is passed to ConfigResolver's constructor (This is currently missing in this draft)

[flangofas]

• Same goes for the AbstractCommand object, setting the Application's configuration object from within a command seems like an anti-pattern, so I did not include it. I did include a getter though because I think it could be needed.

The idea was to provide the ability to commands to change the config object if needed. You are right, it seems anti-pattern indeed.

[l0gicgate]

@ABGEO07

What do you think about Symfony Validator? Using this component we can easily validate the config object.

I'm not opposed to it considering we're already using two symfony packages, it would keep dependencies cohesive. My question here is, how much validation are we going to be doing, if the answer is more then it makes sense. Otherwise I'd like to stay away from including too many librairies.

I want to pick your brain about global usage, how do we detect global usage sanely? I'm guessing we would use something like getcwd() to establish the rootDir right?

[l0gicgate]

@flangofas

a makes more sense to me because then config directory can be resolved and used also as project's root directory which is passed to ConfigResolver's constructor (This is currently missing in this draft)

I’m conflicted as to what we should do in this regard. I would like to explore the option of doing both. But what would installing this globally provide to the end user? What features require global scope.

[ABGEO]

@l0gicgate

Otherwise I'd like to stay away from including too many librairies.

You are right.

I'm guessing we would use something like getcwd() to establish the rootDir right?

I like the idea of using getcwd(). This will help us if we want to initialize a new project in the current directory using the Slim Console global installation. For example ~/.composer/vendor/bin/slim init [new-project-name] will create a new folder new-project-name and initialize an empty project.

[adriansuter]

Maybe I have missed something, but is there a reason we don't use the Symfony Config component? I mean, we have it in composer.json.

[l0gicgate]

@adriansuter

Do you think that it is an overkill to define some sort of ConfigParserInterface and then for example PhpConfigParser and JsonConfigParser?

I think that's a great idea!

Maybe I have missed something, but is there a reason we don't use the Symfony Config component? I mean, we have it in composer.json.

I've been pondering over that. I initially added that dependency but would we be able to get away without it? I'm always in favor of one less dependency if possible. It's also worth mentioning that the minimum PHP version fro symfony/config is 7.2.5

[l0gicgate]

@ABGEO07

For example ~/.composer/vendor/bin/slim init [new-project-name] will create a new folder new-project-name and initialize an empty project.

I'm curious though beside initializing an empty project from anywhere. What other application would this have globally?

Realistically it only saves one step:

composer global require slim/console
php ~/.composer/vendor/bin/slim-console init

composer init
composer require slim/console
php ./vendor/bin/slim-console init

Are we adding needless complexity just for the sake of being able to install this globally? I'm open to still doing it, I'd like to have more than just one command benefiting from that in order to proceed with this though.

[flangofas]

I’m conflicted as to what we should do in this regard. I would like to explore the option of doing both. But what would installing this globally provide to the end user? What features require global scope.

@l0gicgate I am excited about the initial feature set, this is a good and slim start for now.

Having the SlimConsole available globally would improve the installation process, I guess. This means you can install fast a project or prototype, choose a skeleton (this is a good option for newcomers), add additional (dev) dependencies would be nice too, and also any Slim libraries or tools out there.

[l0gicgate]

@adriansuter I did a little bit more thinking about this and we have a small problem. If we init a project in an empty directory, there won't be a configuration file at all since we're going to do it via the interactive process. In this case I'm thinking that the flow will be:

Try resolve from env -> Try resolve from file -> Create from defaults

I will be adding a Config::fromDefaults() method to solve that problem.

Thoughts?

[l0gicgate]

@adriansuter I just wrote all the tests for this. It's a pretty big PR, sorry.

Please review when you have time.

[adriansuter]

@l0gicgate Config::fromDefaults() is a good solution.

I will try to find some time.

[l0gicgate]

Wow we did it y'all! Thank you @adriansuter @ABGEO @flangofas. First piece of the puzzle is getting merged!