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

[bundle] Renamed exposed services' name to classes' FQCN #242

Merged
merged 1 commit into from
Oct 30, 2017
Merged

[bundle] Renamed exposed services' name to classes' FQCN #242

merged 1 commit into from
Oct 30, 2017

Conversation

Lctrs
Copy link
Contributor

@Lctrs Lctrs commented Oct 26, 2017

No description provided.

@makasim
Copy link
Member

makasim commented Oct 26, 2017

@Lctrs
Copy link
Contributor Author

Lctrs commented Oct 26, 2017

No. It means that bundles should configure themselves their internal and exposed services. Here, those new aliases are meant to allow users of Enqueue to autowire Enqueue's bundle's services into their own classes in a Symfony app.

@makasim
Copy link
Member

makasim commented Oct 26, 2017

So the bundle may contain autowiring declarations but it shouldn't use them itself, right?

@Lctrs
Copy link
Contributor Author

Lctrs commented Oct 26, 2017

Yep. Anyway, by declaring our services with their arguments in a config file as it is done actually, this bundle don't use autowiring actually.

But by not exposing the public services by their FQCN, they could not be autowired by Symfony 4 in users' services.
Currently, we are getting deprecation messages like this in a Symfony 3 app :
Autowiring services based on the types they implement is deprecated since Symfony 3.3 and won't be supported in version 4.0. You should rename (or alias) the "enqueue.client.producer" service to "Enqueue\Client\Producer" instead.

@Lctrs
Copy link
Contributor Author

Lctrs commented Oct 26, 2017

An alternative could be to deprecate all enqueue.client.producer-like service names and aliases and promote FQCN as service name instead.

@Lctrs
Copy link
Contributor Author

Lctrs commented Oct 26, 2017

I was thinking as something like this :

services:
    Enqueue\Client\Producer:
        class: 'Enqueue\Client\Producer'
        arguments:
            - '@enqueue.client.driver'
            - '@enqueue.client.rpc_factory'
            - '@enqueue.client.extensions'

    enqueue.client.producer:
        deprecated: The "%service_id%" service is deprecated since 0.9 and will be removed in 1.0. Use "Enqueue\Client\Producer" instead.
        alias: Enqueue\Client\Producer

But it looks like it's not supported by Symfony sadly (symfony/symfony#24507).

@makasim
Copy link
Member

makasim commented Oct 26, 2017

I was thinking as something like this :

looks good. let's do it.

The message should be:
The "%service_id%" service is deprecated since 0.9 and will be removed in 0.10. Use "Enqueue\Client\Producer" instead.

@makasim
Copy link
Member

makasim commented Oct 26, 2017

We could go without depreciation notice. I am fine with it. Leave there a yaml comment there.

@Lctrs Lctrs changed the title Added some alias for autowiring in Symfony 4.0 Renamed exposed services' name to classes' FQCN Oct 26, 2017
@Lctrs Lctrs changed the title Renamed exposed services' name to classes' FQCN [WIP][bundle] Renamed exposed services' name to classes' FQCN Oct 26, 2017
@Lctrs Lctrs changed the title [WIP][bundle] Renamed exposed services' name to classes' FQCN [bundle] Renamed exposed services' name to classes' FQCN Oct 26, 2017
@Lctrs
Copy link
Contributor Author

Lctrs commented Oct 26, 2017

@makasim If tests are green, it's ready to be reviewed.

@makasim
Copy link
Member

makasim commented Oct 27, 2017

@Lctrs still red

@Lctrs
Copy link
Contributor Author

Lctrs commented Oct 27, 2017

Yep saw it.

Lowercased services identifiers in Symfony < 3.3 is annoying. I don't have time today to work on it. I'll try to fix it by monday.

@Lctrs
Copy link
Contributor Author

Lctrs commented Oct 30, 2017

@makasim Failures are unrelated.

@makasim makasim merged commit 2a6f217 into php-enqueue:master Oct 30, 2017
@Lctrs Lctrs deleted the service-alias branch October 30, 2017 16:12
ASKozienko pushed a commit that referenced this pull request Nov 2, 2018
[bundle] Renamed exposed services' name to classes' FQCN
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants