Skip to content

Conversation

@fabpot
Copy link
Member

@fabpot fabpot commented Aug 31, 2014

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

The NotFoundActivationStrategy class from MonologBundle is totally independent of the framework and should be part of the bridge instead. That would allow people to use it easily with Silex for instance.

ping @Seldaek

@fabpot
Copy link
Member Author

fabpot commented Aug 31, 2014

Also, would it makes sense to make this class more "flexible" like explained in symfony/monolog-bundle#89?

ping @lyrixx

@fabpot
Copy link
Member Author

fabpot commented Aug 31, 2014

see symfony/monolog-bundle#94 for the bundle counterpart.

@fabpot
Copy link
Member Author

fabpot commented Aug 31, 2014

Just noticed that the class from the bundle uses the request instead of the request stack. I've made a second commit to use the request stack for the class in the bridge.

@fabpot fabpot force-pushed the notfound-activation-strategy branch from 3491751 to bf5e9d1 Compare August 31, 2014 06:20
Copy link
Member

Choose a reason for hiding this comment

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

Maybe best to move this inline in the if, or at least return early if $isActivated is false, to avoid getting the request every time? Otherwise the PR and RequestStack changes seem fine to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@fabpot fabpot force-pushed the notfound-activation-strategy branch from bf5e9d1 to b064d2f Compare September 6, 2014 10:17
@Seldaek
Copy link
Member

Seldaek commented Sep 7, 2014

👍

@fabpot fabpot merged commit b064d2f into symfony:master Sep 8, 2014
fabpot added a commit that referenced this pull request Sep 8, 2014
…gBundle (fabpot)

This PR was merged into the 2.6-dev branch.

Discussion
----------

[Monolog] added NotFoundActivationStrategy from MonologBundle

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

The NotFoundActivationStrategy class from MonologBundle is totally independent of the framework and should be part of the bridge instead. That would allow people to use it easily with Silex for instance.

ping @Seldaek

Commits
-------

b064d2f [Monolog] changed the not found activation strategy to use the request stack
1a239af [Monolog] added NotFoundActivationStrategy from MonologBundle
@fabpot fabpot deleted the notfound-activation-strategy branch September 8, 2014 12:49
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.

2 participants