search

minkphp / MinkGoutteDriver

Goutte driver for Mink framework
MIT License
299 stars 52 forks source link

Update to use Goutte 3.0 #59

Closed hussainweb closed 9 years ago

hussainweb commented 9 years ago

Goutte 3.0 was recently released to support Guzzle 6.0. Since Drupal 8 is moving to Guzzle 6, and hence Goutte 3.0, we also need a corresponding release of MinkGoutteDriver that can work with them.

hussainweb commented 9 years ago

I have filed the issue with the test failure in Guzzle queue at https://github.com/guzzle/guzzle/issues/1139. Depending on the answer there, we will be able to proceed here.

hussainweb commented 9 years ago

Okay, tests pass on PHP 5.3 and 5.4. It seems that composer downloads the older Goutte libraries (and hence older Guzzle) for < PHP 5.5. This means all is well with this PR, as soon as the test failure is fixed in Guzzle

csarrazi commented 9 years ago

Should be fixed, now!

hussainweb commented 9 years ago

Thanks @csarrazi for the fix in Goutte. In case you haven't seen, the issue in Guzzle was closed with a documentation update.

hussainweb commented 9 years ago

It passed on all PHP versions but fails on HHVM due to a missing constant CURLOPT_PROTOCOLS. I have created a PR for this for guzzle at https://github.com/guzzle/guzzle/pull/1143.

csarrazi commented 9 years ago

Saw that. I also submitted a PR to fix the documentation, which is still wrong in the quick start page.

hussainweb commented 9 years ago

The failure is only with HHVM. I am working on fixing those in Guzzle and making some progress, but there is a lot to do. Might I suggest to merge this anyway and create a release as it is stopping Drupal 8 from adopting Guzzle 6 (https://www.drupal.org/node/2493911).

From what I understand, the current version of Guzzle used (3.x) doesn't support HHVM as well. I am not sure if we are proactively supporting hhvm in MinkGoutteDriver. Thoughts please?

csarrazi commented 9 years ago

@stof We should allow failures for HHVM, for MinkGoutteDriver. What's your thought on this?

By the way, @hussainweb, did you see whether there was something regarding that issue in the hhvm project?

hussainweb commented 9 years ago

@csarrazi, on hhvm, no. The fix was simple and everything from PHP 5.3 onwards behaved consistently. I will try finding an issue in HHVM anyway.

sskorc commented 9 years ago

:+1:

hussainweb commented 9 years ago

Thanks @stof. Are you planning a release with this sometime soon?

@csarrazi, BTW, I created an issue in HHVM for one of the issues at https://github.com/facebook/hhvm/issues/5541. This is just FYI.