Skip to content

deprecated the activation strategy class in favor of the one in Symfony #94

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from
Closed

deprecated the activation strategy class in favor of the one in Symfony #94

wants to merge 1 commit into from

Conversation

fabpot
Copy link
Member

@fabpot fabpot commented Aug 31, 2014

In symfony/symfony#11805, I propose to move the activation strategy class to the Symfont bridge. This PR deprecates the current class from the bundle in favor of the one in the bridge. Obviously, this PR can not be merged before the release of Symfony 2.6 at the earliest.

@fabpot
Copy link
Member Author

fabpot commented Aug 31, 2014

I've kept the old class to avoid BC breaks, but we can also remove it altogether, depending on what we prefer and the version of the bundle it will be merged in.

@fabpot
Copy link
Member Author

fabpot commented Aug 31, 2014

As I've migrated the class in Symfony to use the request stack, this PR won't work anymore. Let's see what happens to the PR for Symfony before deciding on the best strategy for the migration here.

@stof
Copy link
Member

stof commented Aug 31, 2014

We should keep the support of older symfony versions IMO. Otherwise we will be forced to maintain several branches of the bundle in parallel because of the 2.3 LTS.

@Seldaek
Copy link
Member

Seldaek commented Sep 5, 2014

+1 on 2.3 compat if possible, we can use the Symfony class if it's available and the bundle one otherwise, that way if some code is out of sync by accident the latest symfony code gets used at least.

@fabpot
Copy link
Member Author

fabpot commented Sep 8, 2014

Not sure how to make things compatible and not break BC. Detecting which class to use can only be done on runtime as the user might have overridden the activation strategy class parameter. But then, the way to create the strategy is not the same (request vs request_stack.)

The only way I can see is to create a factory that will create the strategy the right way, but honestly, that seems overkill to me.

@Seldaek
Copy link
Member

Seldaek commented Sep 9, 2014

Yeah that's true. I guess we have to tag a 2.6+ release when that's out then..

@Seldaek Seldaek added this to the 3.0 milestone Dec 28, 2014
@jum4
Copy link

jum4 commented Apr 13, 2016

Hi,
What about this PR ?
'request' was removed in Symfony3 so NotFoundActivationStrategy does not work anymore.

@Seldaek
Copy link
Member

Seldaek commented Apr 13, 2016

I guess at some point we need to branch out for symfony 3.0 (or 2.7+) into a v3.0 of this bundle.

@fabpot fabpot closed this Oct 19, 2016
fabpot added a commit that referenced this pull request Oct 19, 2016
This PR was squashed before being merged into the 3.x-dev branch (closes #169).

Discussion
----------

Deprecate the NotFoundActivationStrategy class

Use the version that is in monolog-bridge instead. That version uses
request_stack rather than request from the service container, which
means excluded_404s shall be properly ignored in Symfony 3.

Replaces #94, Fixes #166 and invalidates #123.

This change remains backwards compatible with Symfony 2.6 and above.

Commits
-------

b77bdf0 Deprecate the NotFoundActivationStrategy class
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants