[Maintenance] Make all API Bundle services public and unify public notation #12974
Conversation
lchrusciel
commented
Aug 18, 2021
lchrusciel
commented
| Q | A |
|---|---|
| Branch? | master |
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Related tickets | |
| License | MIT |
lchrusciel
added
Enhancement
|
Hmmmmmmm, with all the DI injection, autowiring etc. why such change? It's against best practices of Symfony IIRC. |
|
@lchrusciel Could you give more background on why we want to have this? |
lchrusciel
force-pushed
the
public-api-bundle
branch
from
7cf08b0
to
280519e
Compare
|
You are right, this PR is against Symfony Best practices. However, our internal rule is to make all services public, due to Sylius Resource Controller and testing with PHPUnit for example. In all these scenarios we are fetching services directly from the container. In the resource controller, the services are fetched to perform regular logic. In the case of PHPUnit, I just had a failing test at #12968. IIRC, Behat also requires us to have public services, but I'm not 100% sure about it. These changes should make us coherent with something, that we introduced in #9366. All our services should be public by default, unless we will write a new ADR |
lchrusciel
force-pushed
the
public-api-bundle
branch
from
280519e
to
b06ca48
Compare
|
Thank you, Łukasz! 🎉 |
…ner by default (lchrusciel) This PR was merged into the 1.11-dev branch. Discussion ---------- | Q | A | --------------- | ----- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | #13230 (comment) | License | MIT All services in Sylius are made public by default. This approach is needed for the potential ResourceController execution, WinzouStateMachine usage, as well as for the ease of testing. Already explained in #12974 (comment). <!-- - Bug fixes must be submitted against the 1.9 or 1.10 branch (the lowest possible) - Features and deprecations must be submitted against the master branch - Make sure that the correct base branch is set To be sure you are not breaking any Backward Compatibilities, check the documentation: https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html --> Commits ------- 4cb828a [CatalogPromotion] Make criteria services public in container by default
