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 #279

Merged
merged 8 commits into from
Dec 8, 2022

Conversation

loic425
Copy link
Member

@loic425 loic425 commented Nov 14, 2022

No description provided.

Comment on lines 30 to 33
<service id="sylius.grid.array_to_definition_converter" class="Sylius\Component\Grid\Definition\ArrayToDefinitionConverter">
<argument type="service" id="event_dispatcher" />
</service>
<service id="Sylius\Component\Grid\Definition\ArrayToDefinitionConverterInterface" alias="sylius.grid.array_to_definition_converter" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you flip the alias when there's an interface?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need to keep the previous id and add the new service id with the interface.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I get that, the thing I had in mind is why not go

<service id="Sylius\Component\Grid\Definition\ArrayToDefinitionConverterInterface" class="Sylius\Component\Grid\Definition\ArrayToDefinitionConverter">
    <argument type="service" id="event_dispatcher" />
</service>
<service id="sylius.grid.array_to_definition_converter" alias="Sylius\Component\Grid\Definition\ArrayToDefinitionConverterInterface" />

so the new id is always in the service definition and the old service name is aliased.

Copy link
Member Author

Choose a reason for hiding this comment

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

lchrusciel and others added 3 commits November 25, 2022 15:26
This PR was merged into the 1.12 branch.

Discussion
----------



Commits
-------

2e24d47 Fix the build
@loic425 loic425 changed the title Add services aliases Add services aliases to improve autowiring Nov 28, 2022
@lchrusciel lchrusciel changed the base branch from 1.12 to 1.13 December 8, 2022 14:44
@lchrusciel lchrusciel added Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). DX Issues and PRs aimed at improving Developer eXperience. labels Dec 8, 2022
@lchrusciel lchrusciel merged commit f55fbe0 into Sylius:1.13 Dec 8, 2022
@loic425 loic425 deleted the feature/add-services-aliases branch December 8, 2022 20:05
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.

None yet

3 participants