should autodiscovered clients be public or private?

[dbu]

when building with symfony 3.4, we get warnings that we access private services. but at least some of those are explicitly set to private. what was the intention of creating those alias but then setting them to private?

the other failure is on httplug.client - should that not be public? and what about named clients? should they be public or not? imo the plugins and stuff should be private and only handled in DI, but for the clients themselves, they should be public.

@Nyholm and @fbourigault your names show up in git blame on those lines, what do you think? shall i just remove a bunch of setPublic(false) calls?

[fbourigault]

Those aliases are private as they only exist to wire the httplug.strategy service.

HttplugBundle/Resources/config/services.xml

Lines 7 to 11 in 542d39d

	<service id="httplug.strategy" class="Http\HttplugBundle\Discovery\ConfiguredClientsStrategy">
	<argument type="service" id="httplug.auto_discovery.auto_discovered_client" on-invalid="null"/>
	<argument type="service" id="httplug.auto_discovery.auto_discovered_async" on-invalid="null"/>
	<tag name="kernel.event_subscriber"/>
	</service>

[dbu]

should we do a unit test instead then, with a mock container? or how should we best test this in the future?

[fbourigault]

Adding test to ensure that services are public would help us to prevent BC breaks!

[Nyholm]

I think all services should be private. Why would you ever need $this->contaner->get($service)?

[fbourigault]

For Symfony < 4 compatibility?

[Nyholm]

In Symfony <4 they are public by default. In SF4 they are private as default (as they should)

[Nyholm]

If we do not specify public=xx it would work without any BC break, right?

[fbourigault]

Yes! (I already have a similar discussion on Slack with @dbu).

I'm 👍 for those services being private.

[dbu]

for the record: agreed to keep them at default visibility so they become private in symfony 4.

[Nyholm]

Good!