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

Introduces an Empty ConfigProvider and Module #163

Closed
wants to merge 1 commit into from

Conversation

gsteel
Copy link
Member

@gsteel gsteel commented Apr 6, 2022

Q A
BC Break no
New Feature yes

Description

Adds a Module and ConfigProvider that are effectively empty.

It's not possible to move all of the view helper configuration into the config provider without breaking BC.

Having them at least present and setup in composer.json for the component installer means that consumers will get used to having the provider/module injected into app config, thereby providing a way to introduce factories for various things on the journey to version 3.0

Theses also provide a useful reference point for documenting the expected keys for various things, like view_helpers for helper factories etc and view_helper_config for helper specific options.

Also see #110 for additional context

The introduction of the config provider and module, although largely empty allow consumers to start using them for future compatibility with 3.0

Signed-off-by: George Steel <george@net-glue.co.uk>
@froschdesign
Copy link
Member

froschdesign commented Apr 8, 2022

Having them at least present and setup in composer.json for the component installer means that consumers will get used to having the provider/module injected into app config…

Which is not needed at the moment for an application based on laminas-mvc and a Mezzio application which uses mezzio-laminasviewrenderer. 🤔

@gsteel
Copy link
Member Author

gsteel commented Apr 8, 2022

@froschdesign This pull is about being able to clean things up in future, and as we go with future pull requests. As we introduce factories for various "things" over time, they can be added to the config provider, once it exists. In future, everything in ViewHelperManager can be removed from instance properties into the config provider providing a clean reference point so that consumers can easily figure out what is registered and how it should be constructed. It also allows us to say, for new projects in MVC and Mezzio, or when updating, inject the config providers to make the migration path to 3.0 easier when it comes.

I'm well aware that the mezzio package does not require this stuff, but in my mind, I believe that certain things currently in the mezzio bridge can be brought into view and removed from there…

@froschdesign
Copy link
Member

@gsteel
Everything correct but this not change the fact that module class and the config provider are empty and not needed at the moment, so the pull request can go to a later version than 2.21 or marked as draft.
Or is there something we can add here for version 2.21?

@froschdesign
Copy link
Member

Theses also provide a useful reference point for documenting the expected keys for various things, like view_helpers for helper factories etc and view_helper_config for helper specific options.

You and I might read the code, but the rest look at the documentation. 😉
This must be added to the documentation.

@gsteel
Copy link
Member Author

gsteel commented Apr 8, 2022

@froschdesign
I think I understand your reluctance to add the specifically empty config provider/module. My reasoning is also about attempting to keep pull requests small and focussed - this could easily be added during another pull but it's a lot easier to reason about in isolation. Additionally, in answer to adding in 2.21.x… there are probably forthcoming pulls that can make use of this, maybe not 2.21, but certainly 2.x and most definitely 3.x after a merge-up.

I also appreciate the docs comment too… Having the many various config options "documented" in code in the config provider helps those writing real docs to more easily define exactly how and where configuration options should be changed

@gsteel gsteel added Won't Fix This will not be worked on and removed Enhancement labels Oct 11, 2022
@gsteel
Copy link
Member Author

gsteel commented Oct 11, 2022

Closing this PR - it's stale and of limited value right now particularly because work has started on https://github.com/laminas/laminas-mvc-view

@gsteel gsteel closed this Oct 11, 2022
@gsteel gsteel deleted the feature/empty-config-provider branch October 11, 2022 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Won't Fix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants