Symfony 4 Support

[deluxetom]

No description provided.

[DocRoms]

Symfony 4.0 isn't compatible with old PHP versions (before PHP 7.1 ).

I suggest to:

• Remove version 5.6 and 7.0 on travis.yml file.
  - Add the PHP 7.1 type declarations.
  - Fix atoum tests.
  - Make a new major release for that. (v3.0.0) [Give Up <= PHP 7.0 support; Add SF4 support].

Thank you very much @thomaspicquet for contributing :P

[Oliboy50]

Thanks for this PR @thomaspicquet :)

I agree with @DocRoms, so here is what I expect to see (at least) in this PR:

• Remove versions 5.4, 5.5, 5.6 and 7.0 from travis.yml file

After what I'll approve it :)

[deluxetom]

We just made the changes but the tests are still failing because of the way the container is built with the client, for some reason it can't see any of the new services even if they're public. We tried to add a kernel class since someone recommended it but that didnt fix it.
Let us know if you have any idea how to fix that

[Oliboy50]

@thomaspicquet not too sure about what is failing exactly, but i'd say that some services are not "public" anymore (since SF4 make them "private" by default)

If I understood this failure:

Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "m6_statsd.wildcard_foo". in /home/travis/build/M6Web/StatsdBundle/src/Tests/Units/DependencyInjection/M6WebStatsdExtension.php on line 85

=> tests expect a service "m6_statsd.wildcard_foo" (which is the service id of a "statsd client" service from the fixtures) to be "public" (because we try to access its definition in our tests...), but it's not

I think adding:

$definition->setPublic(true);

here => https://github.com/M6Web/StatsdBundle/blob/master/src/DependencyInjection/M6WebStatsdExtension.php#L156

would fix this test (maybe not all though)

(see https://symfony.com/blog/new-in-symfony-3-4-services-are-private-by-default for more details)

[deluxetom]

Hey sorry for the late reply, we tried to set the service as public but it didnt help. It seems the client during the test doesn't see any service at all.
We're trying to find a solution ;)

[DocRoms]

Hi !
Thank for your reply :p
Do you try to add the

$definition->setPublic(true);

In all the services concerned?

I think you want to add this method here :
https://github.com/M6Web/StatsdBundle/blob/master/src/DependencyInjection/M6WebStatsdExtension.php#L156

Like @Oliboy50 said it,

And here:
https://github.com/M6Web/StatsdBundle/blob/master/src/DependencyInjection/M6WebStatsdExtension.php#L67

Thank you very much to try it :-)

You can also execute the tests in you local environment with this command :

./bin/atoum

(The FAQ has an error... )

[Oliboy50]

LGTM

Thank you :)

I think we won't need to make our services public later, because SF4 is about "dependency injection everywhere", so no one would have to $container->get() our services anymore, but we could remove the setPublic later (for a pretty small performance gain), so it's ok for now :)

👍

[deluxetom]

I agree, it shouldn't have to be public anymore :)

You're welcome, all the credits go to my colleague @tonyh79 for this update.

[Oliboy50]

@thomaspicquet I'll wait for #65 to be merged before bumping a v3.0.0 release containing your PR

thanks again :)

[Oliboy50]

v3.0.0 has been bumped 🎉