phpinsights
phpinsights copied to clipboard
nunomaduro
Feature/add support for custom presets
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| Fixed tickets | #370 |
Adds the ability for custom presets to be added to the configuration as a fully qualified class name.
Yes it should be ready for review. I’m just now seeing merge conflicts, do you want me to resolve them?
So I have no idea why the ComposerLoader class is failing because it's not a class I changed. What do you want me to do about it?
@Andrew-Shook Really sorry for the delay :confounded:
For me, this PR is almost ready :+1: I just have an interrogation if we should throw an exception to user when it has an InvalidPreset in config file, instead of using DefaultPreset. WDYT ?
Oh, and can you also complete the doc to indicate that this feature will be possible in v2 ? :hugs:
@Andrew-Shook Really sorry for the delay 😖
For me, this PR is almost ready 👍 I just have an interrogation if we should throw an exception to user when it has an InvalidPreset in config file, instead of using DefaultPreset. WDYT ?
So I was thinking about this when I did the last change and I do think we should throw an exception if they supply a preset but it’s invalid. I feel like the logic should be: If there is a preset and it’s valid return the preset. If there is a preset and it’s invalid, then throw an exception. If there is no preset, then guess the preset and return the guess. Does that sound good to you?
Oh, and can you also complete the doc to indicate that this feature will be possible in v2 ? 🤗
Is there a v2 of the docs in the repo or should I just add it to the v1 of the docs where configs are covered and mark it as v2?
@Andrew-Shook Sounds good 👍 I agree that, that is the direction we shoud lgo.
You should just mark it with the v2 label 👍
Great idea that was never really followed up on. Closing for now. but has given plenty of food for thought
