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

Symfony 5 and nexylan/slack 3.0 support #34

Merged
merged 4 commits into from Jan 6, 2020

Conversation

@mbabker
Copy link
Contributor

mbabker commented Nov 27, 2019

Also adds PHP 7.3 to the CI configuration

@mbabker
Copy link
Contributor Author

mbabker commented Nov 27, 2019

Well, this ended up becoming more involved than I expected.

Basically, to get around dealing with differing signatures in different versions of matthiasnoback/symfony-dependency-injection-test, I ended up:

  • Raising PHP minimum to 7.3
  • Bumping PHPUnit to 8.x
  • Bumping matthiasnoback/symfony-dependency-injection-test to 4.x

Only the first one in that list will affect bundle users, but considering that version is hours away from EOL at this point I'd say not an issue.

Copy link
Member

soullivaneuh left a comment

Thanks for the work, I appreciate ! 👍

Only the first one in that list will affect bundle users, but considering that version is hours away from EOL at this point I'd say not an issue.

Not our problem, we can easily upgrade your constraints on a new minor according to semver.

And you are right, EOL means "do not use it anymore", after all! 😉

Travis does not seem happy with your new Symfony v5 environment, would you mind to take a look?

phpunit.xml.dist Show resolved Hide resolved
tests/NexySlackBundleTest.php Show resolved Hide resolved
@mbabker
Copy link
Contributor Author

mbabker commented Nov 27, 2019

Travis does not seem happy with your new Symfony v5 environment, would you mind to take a look?

It looks like this is blocked by php-http/HttplugBundle#360, once HttplugBundle updated and has a new release in theory this should be OK.

@mbabker mbabker changed the title Allow use with Symfony 5 Symfony 5 and nexylan/slack 3.0 support Dec 28, 2019
@mbabker
Copy link
Contributor Author

mbabker commented Dec 28, 2019

HttplugBundle finally had a release yesterday. So, bumping this bundle's minimum version of that up to the new version got most things passing. But, the Symfony 5 build still couldn't resolve a full set of dependencies because the nexylan/slack package doesn't support Symfony 5 until the 3.0 release. So, I've bumped that across major versions too and adjusted the Client class configuration to match the needed constructor changes. That also in effect causes this bundle to require PHP 7.3 too. Things should be good to go here now.

@@ -38,6 +38,8 @@ public function getConfigTreeBuilder(): TreeBuilder
->addDefaultsIfNotSet()
->children()
->scalarNode('client')->defaultValue('httplug.client')->end()
->scalarNode('request_factory')->defaultValue('nexy_slack.request_factory.default')->end()
->scalarNode('stream_factory')->defaultValue('nexy_slack.stream_factory.default')->end()

This comment has been minimized.

Copy link
@mbabker

mbabker Dec 28, 2019

Author Contributor

Same logic as with the HTTP client class, allow a bundle user to replace the request and stream factories with their own implementation.

</service>

<service id="nexy_slack.client" alias="Nexy\Slack\Client"/>

<service id="nexy_slack.request_factory.default" class="Psr\Http\Message\RequestFactoryInterface">

This comment has been minimized.

Copy link
@mbabker

mbabker Dec 28, 2019

Author Contributor

HttplugBundle doesn't provide services directly creating the PSR-17 factories, so the services here are the defaults used to discover compatible factories through the Httplug discovery system.

@soullivaneuh soullivaneuh merged commit 7c99a99 into nexylan:master Jan 6, 2020
4 checks passed
4 checks passed
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 99.024%
Details
flintci/pr No issue found, good job!
Details
@soullivaneuh
Copy link
Member

soullivaneuh commented Jan 6, 2020

Thank for your work and your time! 👍

@soullivaneuh soullivaneuh added the patch label Jan 6, 2020
@mbabker mbabker deleted the mbabker:symfony-5 branch Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.