Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle][SecurityBundle] Moved security expression providers pass logic to SecurityBundle#27611
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
HeahDude commentedJun 15, 2018
| Q | A |
|---|---|
| Branch? | master |
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | ~ |
| License | MIT |
| Doc PR | ~ |
stof commentedJun 15, 2018
We probably need to make SecurityBundle conflict with older versions of FrameworkBundle to ensure we don't end up with double registration |
| */ | ||
| publicfunctionprocess(ContainerBuilder$container) | ||
| { | ||
| if ($container->has('security.expression_language')) { |
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.
This can be a guard clause now
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.
If the code remains cleans and small with one indent, I would keep a readable true condition here. I'll wait for other opinions before changing the style.
HeahDude commentedJun 16, 2018
50a09da to0869588Compare00631cf to70f6708CompareHeahDude commentedJun 16, 2018
Failures unrelated. Status: needs review |
| "symfony/var-dumper":"<3.4", | ||
| "symfony/event-dispatcher":"<3.4", | ||
| "symfony/framework-bundle":"<4.1.1", | ||
| "symfony/framework-bundle":"<4.2", |
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.
L33 should bump to 4.2 also
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.
Fixed.
| publicfunction__construct(bool$handleSecurityLanguageProviders =true) | ||
| { | ||
| if ($handleSecurityLanguageProviders) { | ||
| @trigger_error(sprintf('Registering services tagged with "security.expression_language_provider" with "%s" is deprecated since Symfony 4.2, use the "%s" instead.',__CLASS__, SecurityExpressionLanguageProvidersPass::class),E_USER_DEPRECATED); |
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'd suggest removing the first "with".
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.
Agreed, done.
| * Allowed configuring taggable cache pools via a new`framework.cache.pools.tags` option (bool|service-id) | ||
| * Deprecated auto-injection of the container in AbstractController instances, register them as service subscribers instead | ||
| * Deprecated the processing of services tagged`security.expression_language_provider` in favor of the new SecurityBundle pass |
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.
Deprecated processing of services taggedsecurity.expression_language_provider in favor of a newAddExpressionLanguageProvidersPass in SecurityBundle.
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.
Done.
… pass logic to SecurityBundle
70f6708 toa8017beComparenicolas-grekas commentedJun 19, 2018
Thank you@HeahDude. |
…ssion providers pass logic to SecurityBundle (HeahDude)This PR was merged into the 4.2-dev branch.Discussion----------[FrameworkBundle][SecurityBundle] Moved security expression providers pass logic to SecurityBundle| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | ~| License | MIT| Doc PR | ~Commits-------a8017be [FrameworkBundle][SecurityBundle] Moved security expression providers pass logic to SecurityBundle
This PR was merged into the 4.2-dev branch.Discussion----------[FrameworkBundle] Fixed tests added in#27611| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no <!-- don't forget to update src/**/CHANGELOG.md files -->| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets | ~| License | MIT| Doc PR | ~<!--Write a short README entry for your feature/bugfix here (replace this comment block.)This will help people understand your PR and can be used as a start of the Doc PR.Additionally: - Bug fixes must be submitted against the lowest branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against the master branch.-->Commits-------be97006 [FrameworkBundle] Fixed tests added in#27611