Skip to content

[Security] Fix BC break introduced in #10694 #12049

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
Sep 27, 2014

Conversation

romainneutron
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #12034
License MIT

Not sure about this fix, @stof 'ing welcome

@stof
Copy link
Member

stof commented Sep 26, 2014

Fixing the BC break is indeed needed. It is a bit unfortunate that we must make the AuthenticationManager optional, but this is needed for BC anyway

@stof
Copy link
Member

stof commented Sep 26, 2014

👍

@stof
Copy link
Member

stof commented Sep 26, 2014

However, I suspect you forgot to update the tests of the class

@fabpot
Copy link
Member

fabpot commented Sep 27, 2014

I fixed the tests in 2763227

@fabpot fabpot closed this Sep 27, 2014
@fabpot fabpot merged commit b2183aa into symfony:master Sep 27, 2014
fabpot added a commit that referenced this pull request Sep 27, 2014
…ron)

This PR was merged into the 2.6-dev branch.

Discussion
----------

[Security] Fix BC break introduced in #10694

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #12034
| License       | MIT

Not sure about this fix, @stof 'ing welcome

Commits
-------

b2183aa [Security] Fix BC break introduces in #10694
@fabpot
Copy link
Member

fabpot commented Sep 27, 2014

Actually, this fix is wrong :( I fixed it in 4e0021b

@romainneutron romainneutron deleted the fix-12034 branch September 27, 2014 12:38
@romainneutron
Copy link
Contributor Author

thanks @fabpot

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

Successfully merging this pull request may close these issues.

3 participants