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:

• Initial Implementation
• Make decisions
• 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 Implement config object #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]

@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.

[adriansuter]

Wow, that was huge. Great work, @l0gicgate - all the tests are amazing.

By the way, shouldn't your homepage in composer.json have https?

[adriansuter]

@l0gicgate You're a rocket.

Two minor changes (type hints). Other than that, I think we are good to go.

[l0gicgate]

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