Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

• Always add tests
• Keep backward compatibility (see https://symfony.com/bc).
• Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
• Features and deprecations must be submitted against the 6.4 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

Hey!

Thanks for your PR. You are targeting branch "6.4" but it seems your PR description refers to branch "6.4 for features / 5.4 or 6.3 for bug fixes".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

It looks like you unchecked the "Allow edits from maintainer" box. That is fine, but please note that if you have multiple commits, you'll need to squash your commits into one before this can be merged. Or, you can check the "Allow edits from maintainers" box and the maintainer can squash for you.

Cheers!

Carsonbot

How does this behave when the actual request is done with HTTP/2, which does not transmit a reason phrase ?

How does this behave when the actual request is done with HTTP/2, which does not transmit a reason phrase ?

the Regex shouldn't find any, and then return nothing, and then the factory should fallback to the old one

It looks like you unchecked the "Allow edits from maintainer" box. That is fine, but please note that if you have multiple commits, you'll need to squash your commits into one before this can be merged. Or, you can check the "Allow edits from maintainers" box and the maintainer can squash for you.

Cheers!

Carsonbot

Where can I check this right now?

I didn't saw it when making this MR

I'm not sure this checkbox exists for PRs sent from a fork owned by an organization instead of a user.

i probably need to do some trickery with responseFactory->createResponse and the amount of parameters.

while the Response class allows the Reason Phase to be null (and falls back to the hard codes ones, MAYBE)
the factory doesn't allow null as parameter, and passing empty string is not the same as passing 1 parameter.

so i probably need to do something like:

$responseParameters = [$response->getStatusCode()];
if ($phrase = $this->getReasonPhrase($response)) {
  $responseParameters []= $phrase;
}

$psrResponse = $this->responseFactory->createResponse(...$responseParameters);

i need to try this after weekend

This test failure appears to be related.

1) Symfony\Component\HttpClient\Tests\Psr18ClientTest::testInvalidHeaderResponse
TypeError: Http\Discovery\Psr17Factory::createResponse(): Argument #2 ($reasonPhrase) must be of type string, null given, called in /home/runner/work/symfony/symfony/src/Symfony/Component/HttpClient/Psr18Client.php on line 109

/home/runner/work/symfony/symfony/vendor/php-http/discovery/src/Psr17Factory.php:84
/home/runner/work/symfony/symfony/src/Symfony/Component/HttpClient/Psr18Client.php:109
/home/runner/work/symfony/symfony/src/Symfony/Component/HttpClient/Tests/Psr18ClientTest.php:101

That's why I said I need to at this maybe after weekend

This test failure appears to be related.

1) Symfony\Component\HttpClient\Tests\Psr18ClientTest::testInvalidHeaderResponse
TypeError: Http\Discovery\Psr17Factory::createResponse(): Argument #2 ($reasonPhrase) must be of type string, null given, called in /home/runner/work/symfony/symfony/src/Symfony/Component/HttpClient/Psr18Client.php on line 109

/home/runner/work/symfony/symfony/vendor/php-http/discovery/src/Psr17Factory.php:84
/home/runner/work/symfony/symfony/src/Symfony/Component/HttpClient/Psr18Client.php:109
/home/runner/work/symfony/symfony/src/Symfony/Component/HttpClient/Tests/Psr18ClientTest.php:101

took my laptop to work on this thing ... see my idea above with the variable parameters

The last failures seems to be unrelated?

Thank you @Hanmac.

Merged as 115a5a1 in 5.4
For your next PR, please allow edits from maintainers.

For your next PR, please allow edits from maintainers.

Edits were not possible because it was forked to an organization account.