Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[SecurityBundle] resolve class name parameter inside AddSecurityVotersPass#23862

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

Closed
pjarmalavicius wants to merge6 commits intosymfony:3.4frompjarmalavicius:check_voters_validity
Closed

[SecurityBundle] resolve class name parameter inside AddSecurityVotersPass#23862

pjarmalavicius wants to merge6 commits intosymfony:3.4frompjarmalavicius:check_voters_validity

Conversation

pjarmalavicius
Copy link
Contributor

QA
Branch?3.4
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#23733
LicenseMIT

umpirsky reacted with thumbs up emoji
@chalasrchalasr added this to the3.4 milestoneAug 11, 2017
@chalasr
Copy link
Member

Continuing discussion from the fixed ticket:

Yes, it's the same as@chalasr suggests, but if we follow symfony best practices, then I would say that we also should write core to follow it. I mean class name parameters are not recommended and maybe one day it will be deprecated. Having check validity compiler pass doesn't depend on class parameter parsing and it also doesn't change what worked before

Resolving parameter in the existing pass could not break anything, only fix something that currently breaks (since%parameter% aren't valid class names).
Also I'm not fond of having aTYPE_OPTIMIZE pass which doesn't optimize actually.
If we want to fix this, I would say we should be consistent with the existing pass(es) doing it already, resolving parameters in the existing pass.

jvasseur reacted with thumbs up emoji

@chalasr
Copy link
Member

Registering service class names was a convention which has been abandoned, we just stopped recommending to do so, we won't forbid it.

@pjarmalaviciuspjarmalavicius changed the title[SecurityBundle] added compiler pass for checking voters validity[SecurityBundle] resolve class name parameter inside AddSecurityVotersPassAug 21, 2017
if (!is_a($class, VoterInterface::class, true)) {
@trigger_error(sprintf('Using a security.voter tag on a class without implementing the %1$s is deprecated as of 3.4 and will be removed in 4.0. Implement the %1$s instead.', VoterInterface::class), E_USER_DEPRECATED);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

please don't remove the empty lines we have for readability.

@chalasr
Copy link
Member

Thank you@pjarmalavicius.

chalasr pushed a commit that referenced this pull requestAug 21, 2017
…ddSecurityVotersPass (pjarmalavicius)This PR was squashed before being merged into the 3.4 branch (closes#23862).Discussion----------[SecurityBundle] resolve class name parameter inside AddSecurityVotersPass| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#23733| License       | MITCommits-------a86bf52 [SecurityBundle] resolve class name parameter inside AddSecurityVotersPass
This was referencedOct 18, 2017
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@stofstofstof approved these changes

@chalasrchalasrchalasr approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.

4 participants
@pjarmalavicius@chalasr@stof@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp