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

Updated Silex to ~2.0. #43

Merged
merged 1 commit into from
Jul 17, 2017
Merged

Conversation

carl-boisvert
Copy link
Contributor

Silex 2 has been release last year and there was some changes that this library doesn't support.

I don't know if there's a particular reason as why it wasn't updated, but I'm proposing this pull request to update the Silex version to ~2.0.

The ServiceProviderInterface has change namespace in version ~2.0. It's now:
use Pimple\ServiceProviderInterface;

Updated the StatsdServiceProvider to reflect that change

@marcqualie
Copy link
Member

Hey @carl-boisvert thanks for the contribution.

I'm not familiar with Silex 2.0 myself so I can technically verify this, but if this patch works for your app then that should be fine.

One thing I noticed was the tests now fail due to old PHP version requirements. I've updated master to test against the currently supported versions, could you please rebase your branch against that so the tests run?

Thanks

@@ -18,10 +19,9 @@ class StatsdServiceProvider implements ServiceProviderInterface
* Register Service Provider
* @param Application $app Silex application instance
Copy link
Member

Choose a reason for hiding this comment

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

The variable type has now changed, could you please also update the documentation here

The ServiceProviderInterface has change namespace in version ~2.0. It's now:
use Pimple\ServiceProviderInterface;

Updated the StatsdServiceProvider to reflect that change
@carl-boisvert carl-boisvert force-pushed the Update-Silex-Version branch from 13eea1a to 9d9f893 Compare July 17, 2017 16:44
@carl-boisvert
Copy link
Contributor Author

Here we go, rebase done and doc updated!

Thanks for the feedback!

@marcqualie
Copy link
Member

Thanks @carl-boisvert

@marcqualie marcqualie merged commit bee97e7 into thephpleague:master Jul 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants