Merge current 1.x release into master

[bbrala]

I've run into a blocker for us in issue #327 and thought I'd just go through the code and create a merge. This merges #345 and #347 so 2.x branch is up-to-date.

Hopefully this will unblock a 2.x release, since Drupal is waiting for this and out package swisnl/json-api-client.

[andypost]

Great news, thank you! It could use nightly for PHP 8.0 run

[bbrala]

Ok, onbly check that fails is the phpstan one. But not sure how much i should change for these errors.

 ------ -----------------------------------------------------------------------------------
  Line   Message.php
 ------ -----------------------------------------------------------------------------------
  148    Variable $headerLines in PHPDoc tag @var does not match assigned variable $count.
 ------ -----------------------------------------------------------------------------------

 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   Utils.php
 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------
  301    Parameter #1 $fp of function fwrite expects resource, resource|false given.
  301    Parameter #2 $str of function fwrite expects string, bool|float|int|string given.
  302    Parameter #1 $fp of function fseek expects resource, resource|false given.
  304    Parameter #1 $stream of class GuzzleHttp\Psr7\Stream constructor expects resource, resource|false given.
  309    Parameter #1 $stream of class GuzzleHttp\Psr7\Stream constructor expects resource, (callable)|Iterator|Psr\Http\Message\StreamInterface|resource|null given.
  322    Parameter #1 $object of function method_exists expects object|string, (callable)|resource|null given.
  327    Parameter #1 $stream of class GuzzleHttp\Psr7\Stream constructor expects resource, resource|false given.
  353    Parameter #1 $callback of function set_error_handler expects (callable(int, string, string, int, array): bool)|null, Closure(): void given.
  370    Method GuzzleHttp\Psr7\Utils::tryFopen() should return resource but returns resource|false.
  396    Unreachable statement - code above always terminates.
 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------

[bbrala]

There, i've fixed the last phpstan errors, either by some extra checks or adding an ignore to lines where phpstan doesn't completely understand the code.

[GrahamCampbell]

So, just to confirm, there are no calls to the "functions" now, only to "static class methods", just like in the 1.x series. The next step would be to delete the functions entirely from the 2.x series.

[bbrala]

All function calls are removed and I even removed the functions.php itself and the tests. This was so I could verify there was no calls left. So yeah, fully merged and deprecated functions have been removed :) Edit: Mobile typo's

[GrahamCampbell]

Excellent. 👍

[Tobion]

this seems to be missing phpunit 9 update from #350

[bbrala]

@Tobion valid point, will fix that. Seems there was some feedback also on the PR which was disregarded until 2.0, so that would mean some slight extra changes.

[GrahamCampbell]

I think it would be fine for that to be sorted in a follow-up PR. It's more important the other changes from 1.x make it into master than the test suite runs on PHPUnit 9. End users don't use the tests, but they do use what's in src. ;)

[bbrala]

Seems the tests were already merged, but forgot the change in the composer.json and phpunit.xml.dist. Everything is green. I did go though the feedback that was left on the phpunit PR and everything seems as promised :)

[xjm]

I believe the changes requested in #360 (review) have been resolved, correct? @Tobion can you confirm that they've been addressed? Thanks!

[bbrala]

@Tobion hope you had a nice newyears, any chance you can review this any time soon?

[GrahamCampbell]

The HHVM specific code in this PR should be deleted. 2.x does not support HHVM, so the HHVM fixes in this PR should be taken out.

[GrahamCampbell]

See my comment about removing HHVM.

[bbrala]

Sure, i removed the 2 skips for the tests on HHVM.

[bbrala]

Would be nice to get this moving again :)

[barryvdh]

I've looked at this and at first sight it looks good to me. Hard to compare the functions to the static calls though, but the tests are passing so 👍

[Nyholm]

Thank you Barry.

The more people that review and tests this the more confident will the maintainer be to merge it. Your comment is much appreciated.

[GrahamCampbell]

@Nyholm and I have discussed this PR, and it includes unrelated changes and the merges to the utils classes seem to accidentally remove 2.x changes including return types. For this reason, I am going to start again here, so I can be sure everything is correct. Verifying everything in this PR is correct is as much work as me starting again here.

I appreciate everyone's efforts here, and I hope this doesn't put anyone off future contributions. We just have to make sure the merge is good. :)

[Nyholm]

I’m sure you done the best you possible can for this PR. But I cannot be 100% sure of the correctness. I also don’t know you as a contributors.

This might be an issue with my need for control or trust issues, but if you where a person with poor intentions, this is a exactly the kind of PR one should hide bad code in.

We will make sure a version of this PR is merged in the next few days.

[bbrala]

No problem at all. My main goal doing the work was to get this going and merged. I'm glad that is happening soon, even though its not my commit. Thanks for taking the time to get this up and running soon 😀 Outlook for Android<https://aka.ms/ghei36> downloaden

…

________________________________ From: Tobias Nyholm <notifications@github.com> Sent: Sunday, March 7, 2021 4:56:46 PM To: guzzle/psr7 <psr7@noreply.github.com> Cc: Björn Brala <bjorn@swis.nl>; Author <author@noreply.github.com> Subject: Re: [guzzle/psr7] Merge current 1.x release into master (#360) I’m sure you done the best you possible can for this PR. But I cannot be 100% sure of the correctness. I also don’t know you as a contributors. This might be an issue with my need for control or trust issues, but if you where a person with poor intentions, this is a exactly the kind of PR one should hide bad code in. We will make sure a version of this PR is merged in the next few days. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#360 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAZEN6R5EP6VRMGPYPDOIEDTCOO35ANCNFSM4UBC2BQA>.

[xjm]

If there is a new PR, can it be linked? As I understand this was blocking the availability of PSR-17 support in a stable release. Thanks!

[Nyholm]

It is not.

It has been multiple PRs. See the PRs creates in the past few days. Also note that master is already updated with code from 1.x.

[xjm]

Reference for my comment: #327 (comment)

[xjm]

I guess my question is, if this is not the way forward to have PSR-17 support in a stable release, then what is? We need some guidance since #327 has been stalled nearly a year and the last advice we were given here was to try to make the above merge happen.