8000 CS for AccessDecisionManager by aschempp · Pull Request #34991 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

CS for AccessDecisionManager #34991

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

Merged
merged 1 commit into from
Dec 15, 2019
Merged

Conversation

aschempp
Copy link
Contributor
Q A
Branch? 3.4
Bug fix? no
New feature? no
Deprecations? no
Tickets #34548
License MIT
Doc PR -

As discussed in #34548 with @nicolas-grekas here's a CS change for the AccessDecisionManager

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Dec 15, 2019
@chalasr
Copy link
Member
chalasr commented Dec 15, 2019

Thank you @aschempp.

chalasr added a commit that referenced this pull request Dec 15, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

CS for AccessDecisionManager

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | #34548
| License       | MIT
| Doc PR        | -

As discussed in #34548 with @nicolas-grekas here's a CS change for the `AccessDecisionManager`

Commits
-------

b3742ec CS
@chalasr chalasr merged commit b3742ec into symfony:3.4 Dec 15, 2019
@aschempp aschempp deleted the bugfix/access-manager-cs branch December 16, 2019 11:03
@gaetan-petit
Copy link
gaetan-petit commented Feb 10, 2020

Hi all,

This PR breaks old code because of the type check added here :
b3742ec#diff-198f07c52bf3eeb62bd14e3a0fb01dd0L93

Some voters in our application were previously returning true and not 1.

No big deal, but was it intended ? @nicolas-grekas @chalasr

@nicolas-grekas
Copy link
Member

Breaking your code was not intended, but returning true was not supported either...

@gaetan-petit
Copy link

@nicolas-grekas It's code from 2015, nice excuse to clean it up 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0