NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

Change ExceptionListenerPass removeDefinition args#38483

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

Jump to bottom

Closed

adaniloff wants to merge4 commits intosymfony:4.4fromadaniloff:4.4

Closed

Change ExceptionListenerPass removeDefinition args#38483

adaniloff wants to merge4 commits intosymfony:4.4fromadaniloff:4.4

Conversation

Copy link

adaniloff commentedOct 8, 2020

Q	A
Branch?	4.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	Fix#38482
License	MIT
Doc PR

Since the 4.4.15 upgrade, our FOSRest (2.6) configuration did not work anymore.

See the issue in#38482 for more explanation.

Change ExceptionListenerPass removeDefinition args

e78379e

carsonbot added Status: Needs Review Bug labels

Oct 8, 2020

adaniloff mentioned this pull request

Oct 8, 2020

[TwigBundle] ExceptionListenerPass does not seem to call removeDefinition with the right argument#38482

Closed

xabbuh added the TwigBundle label

Oct 9, 2020

xabbuh added this to the4.4 milestone

Oct 9, 2020

adaniloff added3 commits

October 9, 2020 10:47

test: fix class ExceptionListenerPassTest

4c5b1f2

Make the test class so it covers the initial behavior on 4.4.14, prior to the 4.4.15 release.

fix: ExceptionListenerPass definition removal

a1819e7

According the the previous commit; in order to keep it consistent with 4.4.14 version.

style: fix changes style

c322ce0

Copy link

Author

adaniloff commentedOct 9, 2020

So I basically reverted 4.4.15 and I kept the test file.
I then constated that there was a test which was failing:testRemovesTwigExceptionListenerIfTwigIsNotUsedAsTemplateEngine which I skipped.

I then add a test in order to cover the 4.4.14 version current behavior.

Obviously, applying the 4.4.15 changes made my test failed.

The question is:

is that new behavior from 4.4.15 intended ?
if so, then we have an issue because it can breaks backward compatibility in some cases, so I think a patch version is not valid as semantic versionning ?

Curious to know what you guys think about all that.