-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Describe things more precisely #10845
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
Conversation
This was named "PSR-4 based service discovery" in https://symfony.com/blog/new-in-symfony-3-3-psr-4-based-service-discovery, but @weaverryan was worried about people not knowing about what it is in #10689 (comment). Names I've seen so far:
Personally, I've always used the first one. |
Isn't this way to specific? I mean, the thing is: If only one service in the container (be it configured explicitly or through PSR-4 service discovery) that implements this interface, the alias is automatically created.
(I've added the last sentence as a proposal, as I think it's unwanted behavior to have a broken container when creating a new class somewhere in Thanks btw for following up the short slack discussion! |
Right, that is indeed more about the How about "glob-based service declaration", that will trigger a "PSR-4 service discovery".
Are you 100% sure about this? We discussed about this only happening when both the interface and the class are discovered through the same discovery, cc @lucascourot |
Let me try on a fresh install. |
@wouterj when I do this, the alias disappears from
I can publish the repository if you want to tinker with it. Let me take this as an occasion to celebrate how fast it is to create a throwaway Symfony application to prove a point now. Flex is awesome! 🎉 |
I confirm too. It must be discovered by the same discovery in order to work. (see https://github.com/symfony/symfony/blob/5c2cee5c0fcf09d90ffd414bb8d13e023fc5ae9e/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php#L78-L83 which is called for each configured discovery) |
Wow, alrighty. (I'm not really into Symfony core logic anymore 😞) In general, I think people call the What about using the concept name here? "As long as there is only one class implementing the interface, and that class is part of the same PSR-4 service discovery." We can then link the |
Not sure about the PSR-4 part… is this really incompatible with PSR-0? Maybe it only works with PSR-4 for now, but maybe there could be other ways of doing discovery in the future, who knows? How about dropping it? "As long as there is only one class implementing the interface, and that class is part of the same service discovery…" It still feels a bit awkward to use the concept name IMO, especially since the full name is "PSR-4 service discovery and registration". And can we really say "a discovery", or is discovery separated in several parts for which we should find a name ("discovery pass"?) ? |
slack://symfony-devs#naming suggests "{prototype,template} service definition" (credits to @ro0NL). |
@wouterj There are already different ways of breaking things up by adding a new class in src. For example with @greg0ire I'm ok with "prototype service definition" |
Nevertheless, it's a very implicit behavior. Promoting to make this explicit by declaring the alias yourself is a good thing if you ask me. (btw I'm personally never relying on this in a ports & adapters architecture, because I'm used to only use discovery+autowiring for infra & specific conventions for the application layer. This way the interfaces are usually not even part of the same discovery, which once again forces to make everything explicit and is a good thing to me.) @greg0ire : "prototype service definition" is fine. It was the second in my list and it's how the feature is referenced in code & tests. |
Silly me, I didn't even notice! Let's go with that then! Good points about explicit vs implicit, I don't like that implicit autowiring either, but that behavior still needs to be documented properly, and will be challenged more easily if people know it exists. |
Please review again, maybe you can phrase this better than I did. |
Here, namespace is referring to that kind of thing: App\Namespace\: resource: '../../src/App/Namespace/*' And it looks as if there is no word yet to name that kind of configuration block. Let us go with service definition prototype.
Do you consider this PR finished? Thanks. |
If no one can think of a better wording, I do. |
Thank you Grégoire. |
This PR was submitted for the master branch but it was merged into the 3.4 branch instead (closes #10845). Discussion ---------- Describe things more precisely Here, namespace is referring to that kind of thing: App\Namespace\: resource: '../../src/App/Namespace/*' And it looks as if there is no word yet to name that kind of configuration block. I propose to use namespace resource declaration, but please do suggest better alternatives, (glob resouce declaration?). <!-- If your pull request fixes a BUG, use the oldest maintained branch that contains the bug (see https://symfony.com/roadmap for the list of maintained branches). If your pull request documents a NEW FEATURE, use the same Symfony branch where the feature was introduced (and `master` for features of unreleased versions). --> Commits ------- 1a4f8ff Describe things more precisely
Here, namespace is referring to that kind of thing:
And it looks as if there is no word yet to name that kind of
configuration block. I propose to use namespace resource declaration,
but please do suggest better alternatives, (glob resouce declaration?).