Skip to content

[DI] Register PSR4 prototypes definitions #23548

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[DI] Register PSR4 prototypes definitions #23548

wants to merge 1 commit into from

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Jul 17, 2017

Q A
Branch? 3.4
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #23497
License MIT
Doc PR todo

@chalasr chalasr changed the base branch from master to 3.4 July 17, 2017 13:41
@chalasr chalasr force-pushed the psr4-prototype-def branch from 33cc523 to 8c7c214 Compare July 17, 2017 13:41
@chalasr chalasr mentioned this pull request Jul 17, 2017
Closed
@nicolas-grekas
Copy link
Member

Would that be usable with the default config, or will child definitions conflict?
A functional test ensuring this works would be great :)

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Jul 17, 2017
@james-johnston-thumbtack james-johnston-thumbtack mentioned this pull request Jul 17, 2017
Merged
@chalasr chalasr force-pushed the psr4-prototype-def branch from 8c7c214 to d3d4e28 Compare July 19, 2017 11:24
@chalasr chalasr force-pushed the psr4-prototype-def branch from d3d4e28 to 53f2b5a Compare July 19, 2017 11:26
@chalasr chalasr requested a review from nicolas-grekas July 19, 2017 13:19
nicolas-grekas
nicolas-grekas reviewed Jul 19, 2017
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This makes one able to inherit from one specific prototype service and specialize it manually.
But the inheritance is limited by the fact that all attributes in _defaults and all tags must be repeated since they are not inherited.
This means the interest of inheriting from a prototype is restricted to all the other possible attributes (setters, etc.)
If this is enough value, then implementation looks good to me. If not, it is worth it? Then should we seek for another syntax? That's the questions in my head right now :)

@chalasr
Copy link
Member Author

chalasr commented Jul 19, 2017

Writing the integration test made me feel it's not worth it, the code in the fixed ticket would be as verbose as if it was using the current possible alternatives. More limitations and edge cases to cover than added value.
I think parent is still useful for distributed stuff but should now be very rare in userland, and is surely not the right way to improve the situation.

If no better solution (I have no) then we can close here

@nicolas-grekas
Copy link
Member

OK, closing, same feeling. We can still reconsider if a better idea comes up of course. Thanks for trying.

@chalasr chalasr deleted the psr4-prototype-def branch July 19, 2017 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

Assignees
No one assigned
Labels
DependencyInjection Feature Status: Needs Review
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.
3 participants
@chalasr @nicolas-grekas @carsonbot