-
Notifications
You must be signed in to change notification settings - Fork 203
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
EZP-31046: Improved ChainConfigResolver by extracting code to separate config resolvers #2901
Conversation
a1f03cc
to
c6a0d73
Compare
...lishCoreBundle/DependencyInjection/Configuration/ConfigResolver/SiteAccessConfigResolver.php
Show resolved
Hide resolved
eZ/Bundle/EzPublishCoreBundle/DependencyInjection/EzPublishCoreExtension.php
Show resolved
Hide resolved
...shCoreBundle/DependencyInjection/Configuration/ConfigResolver/DefaultScopeConfigResolver.php
Show resolved
Hide resolved
...ishCoreBundle/DependencyInjection/Configuration/ConfigResolver/GlobalScopeConfigResolver.php
Show resolved
Hide resolved
...oreBundle/DependencyInjection/Configuration/ConfigResolver/SiteAccessGroupConfigResolver.php
Show resolved
Hide resolved
...reBundle/DependencyInjection/Configuration/ConfigResolver/StaticSiteAccessConfigResolver.php
Show resolved
Hide resolved
4a4f0d4
to
8dab852
Compare
8dab852
to
4492898
Compare
...blishCoreBundle/DependencyInjection/Configuration/ConfigResolver/ContainerConfigResolver.php
Outdated
Show resolved
Hide resolved
...lishCoreBundle/DependencyInjection/Configuration/ConfigResolver/SiteAccessConfigResolver.php
Outdated
Show resolved
Hide resolved
...lishCoreBundle/DependencyInjection/Configuration/ConfigResolver/SiteAccessConfigResolver.php
Outdated
Show resolved
Hide resolved
...lishCoreBundle/DependencyInjection/Configuration/ConfigResolver/SiteAccessConfigResolver.php
Outdated
Show resolved
Hide resolved
0492b5b
to
1333a1d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I quite like the single-responsibility approach here. Good job 🎉
squash! EZP-31046: Implemented ChainConfigResolver
1333a1d
to
1eaec10
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QA Approved on eZ Platform EE master with branch.
Issue with CT icons resolved.
Verified with regression suite and sanities.
master
This PR improves ChainConfigResolver by extracting code to separate config resolvers (default, site access, site access group and global resolver). In addition, it adds type hints to Configresolver, removed Site Access relation handling (which is replaced by a dedicated method in SiteAccessService in the previous PR).
TODO:
$ composer fix-cs
).