Switch to PSR18 client implementation and bump httplug minimum version to ^2.0

[GrahamCampbell]

php-http/client-implementation is deprecated and Guzzle 7 doesn't implement it, so this will be a blocker for anyone wanting to upgrade from Guzzle 6 to 7 and use this package at the same time.

---

Related: GitLabPHP/Client#511, BitbucketPHP/Client#39, php-http/discovery#175.

[acrobat]

Thanks for the PR @GrahamCampbell! So in a nutshell this would mean dropping httplug v1 support as v2 already supports psr-18?

Can we also remove the other bc layers then?

php-github-api/lib/Github/HttpClient/Plugin/PathPrepend.php

Line 15 in b3a7252

use Plugin\VersionBridgePlugin;

Can you also check the failing tests? Thanks!

[GrahamCampbell]

Yes and yes. :)

[GrahamCampbell]

This PR is ready now. :)

[acrobat]

Thank you @GrahamCampbell

[acrobat]

@GrahamCampbell this actually breaks the current library. I've setup a fresh project with the install instructions for php 7.2 from the readme and while creating the client I get this error:

PHP Fatal error:  Uncaught Http\Discovery\Exception\DiscoveryFailedException: Could not find resource using any discovery strategy. Find more information at http://docs.php-http.org/en/latest/discovery.html#common-errors
 - Puli Factory is not available
 - No valid candidate found using strategy "Http\Discovery\Strategy\CommonClassesStrategy". We tested the following candidates: Http\Discovery\Strategy\CommonClassesStrategy::symfonyPsr18Instantiate, Http\Discovery\Strategy\CommonClassesStrategy::buzzInstantiate.
 - No valid candidate found using strategy "Http\Discovery\Strategy\CommonPsr17ClassesStrategy". We tested the following candidates: .

[acrobat]

Hmm I see your issue in php-http/discovery#174. I'm thinking on reverting the merge commit for now, so you can create a new PR with the changes again. This way we can test the new setup separate from master (including the BC breaks suggested in #887) and only merge it when all the upstream packages are fixed.

[GrahamCampbell]

No need to revert. You can still use Guzzle 6. Use the instructions for PHP 7.1.

[GrahamCampbell]

We could potentially change the README of this repo to recommend Guzzle 6 only, for now? Then change the instructions back after the discovery package has been fixed. It was really an oversight for us to release Guzzle 7 without checking the PHP HTTP discovery could pick it up tbh.

[acrobat]

Yes indeed that's the only thing that currently not a great user experience, so that would be great! Can you send a PR?

[GrahamCampbell]

#889

[acrobat]

Thanks!
I tested the master branch on an existing project but it errors during composer update because there is no psr/http-client-implementation package installed. I'm more in favor of reverting the merge, as after fixing locally the roave bc checker (pr will come soon) bc breaks were reported, so the master branch is clean again and you can recreate this PR against the v3-dev branch were already some v3 work was done.

This will give us some time to better test these changes and indeed do the other deprecation cleanups there. Also write clear instruction on how to upgrade. When everything is ready we can merge this branch into master for a 3.0 release.

[GrahamCampbell]

Why not pull out a 2.x branch and make it the default branch, and mark master as 3.x-dev?

[GrahamCampbell]

From what I can see, the v3-dev branch can be discarded, because the only changes in there are outdated now, and were already done in 2.x?

[acrobat]

You were right, 2.x branch is created and is now the default branch, master is the upcoming 3.0 version.

[GrahamCampbell]

Awesome. 👍