[FrameworkBundle] Add missing autowiring aliases for common interfaces #21517
Conversation
chalasr
commented
Feb 2, 2017
•
| Q | A |
|---|---|
| Branch? | master |
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | - |
| License | MIT |
| Doc PR | n/a |
113262c to
e283263
Compare
e283263 to
c3a2141
Compare
|
should we add LoggerInterface also? or do we consider it should be done on monolog bundle? |
Not sure but I would say yes so each bundle is responsible for supporting autowiring for its services |
|
Thank you @chalasr. |
…ommon interfaces (chalasr) This PR was merged into the 3.3-dev branch. Discussion ---------- [FrameworkBundle] Add missing autowiring aliases for common interfaces | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | n/a Commits ------- c3a2141 [FrameworkBundle] Add more autowiring aliases
|
|
||
| return array( | ||
| new FrameworkBundle(), | ||
| new SecurityBundle(), |
There was a problem hiding this comment.
Why is this test located in the FrameworkBundle and not in the SecurityBundle?
There was a problem hiding this comment.
@xabbuh I thought about that but it was simpler to integrate a new test here than duplicating the whole test case on the SecurityBundle. Does it hurt?
There was a problem hiding this comment.
I missed that, but indeed, that's not good. @chalasr Can you move it to SecurityBundle?
|
@nicolas-grekas See symfony/monolog-bundle#203 for LoggerInterface |
|
I would also ensure that |
This PR was merged into the 3.3-dev branch. Discussion ---------- [SecurityBundle] Add AutowiringTypesTest | Q | A | ------------- | --- | Branch? | master | Tests pass? | yes | Fixed tickets | #21517 (comment) | License | MIT Commits ------- d1001d4 [SecurityBundle] Add AutowiringTypesTest