Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add services' aliases to improve autowiring experience #498

Merged
merged 3 commits into from
Nov 28, 2022

Conversation

loic425
Copy link
Member

@loic425 loic425 commented Nov 14, 2022

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Related tickets
License MIT

@loic425 loic425 requested a review from a team as a code owner November 14, 2022 09:46
@loic425 loic425 force-pushed the feature/add-services-aliases branch from 96d0aaa to 95fd4c1 Compare November 25, 2022 14:30
@Zales0123 Zales0123 added the DX Issues and PRs aimed at improving Developer eXperience. label Nov 25, 2022
@@ -19,40 +19,69 @@
<argument type="service" id="service_container" />
<argument type="service" id="sylius.expression_language" />
</service>
<service id="Sylius\Bundle\ResourceBundle\Controller\ParametersParserInterface" alias="sylius.resource_controller.parameters_parser" public="false" />
Copy link
Member

Choose a reason for hiding this comment

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

Can we reverse this definition? So the interface will be on a service itself and sylius.resource_controller.parameters_parser will alias to it? Same as requested: Sylius/SyliusGridBundle#279 (comment)

@lchrusciel lchrusciel added Feature New feature proposals. Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). and removed Feature New feature proposals. labels Nov 28, 2022
@lchrusciel lchrusciel merged commit 4c18512 into Sylius:1.10 Nov 28, 2022
@loic425 loic425 deleted the feature/add-services-aliases branch November 28, 2022 17:12
<argument type="service" id="sylius.resource_registry" />
<tag name="console.command" />
</service>
<service id="sylius.console.command.resource_debug" alias="Sylius\Bundle\ResourceBundle\Command\DebugResourceCommand" />
Copy link
Contributor

Choose a reason for hiding this comment

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

@loic425 @lchrusciel Changes like this one could technically be a B/C break. You're changing a service ID from a definition to an alias. Granted, with this specific case, it's probably not a big deal, but if someone were doing something in a compiler pass, a $container->getDefinition('sylius.console.command.resource_debug') call would no longer work.

GSadee added a commit that referenced this pull request Jan 4, 2023
This PR was merged into the 1.10 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Bug fix?        | yes
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets |
| License         | MIT

Replace #549 

Fix the BC-break from #498 

Commits
-------

947dc82 Flip back service ids and FQCN
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Issues and PRs aimed at improving Developer eXperience. Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants