Skip to content

Conversation

saundefined
Copy link
Member

@saundefined saundefined commented Sep 25, 2025

Copy link
Contributor

github-actions bot commented Sep 25, 2025

🚀 Regression report for commit 55be0cb is at https://web-php-regression-report-pr-1454.preview.thephp.foundation

Copy link
Contributor

github-actions bot commented Sep 25, 2025

🚀 Preview for commit 55be0cb can be found at https://web-php-pr-1454.preview.thephp.foundation

Comment on lines 295 to 298
$versions = ['PHP 8.1', 'PHP 8.2', 'PHP 8.3', 'PHP 8.4', 'PHP 8.5'];
var_dump($versions[array_key_first($versions)]);
// string(7) "PHP 8.1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this example too simple to be useful, especially because $versions[0] works for a list. Perhaps we can find something with an associative array?

Comment on lines +382 to +388
$upcomingRelease = null;
foreach ($php as $key => $version) {
if ($version['state'] === 'upcoming') {
$upcomingRelease = $version;
break;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a good example, because this could just be array_find().

Comment on lines +409 to +414
$upcomingRelease = array_first(
array_filter(
$php,
static fn($version) => $version['state'] === 'upcoming'
)
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same.

Comment on lines 306 to 358
<div class="php8-compare__main">
<div class="php8-compare__block example-contents">
<div class="php8-compare__label">PHP &lt; 8.5</div>
<div class="php8-code phpcode">
<?php highlight_php_trimmed(
<<<'PHP'
$sh = curl_share_init();
curl_share_setopt($sh, CURLSHOPT_SHARE, CURL_LOCK_DATA_DNS);
curl_share_setopt($sh, CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT);
$ch1 = curl_init('https://php.net/');
curl_setopt($ch1, CURLOPT_SHARE, $sh);
curl_exec($ch1);
$ch2 = curl_init('https://thephp.foundation/');
curl_setopt($ch2, CURLOPT_SHARE, $sh);
curl_exec($ch2);
curl_share_close($sh);
curl_close($ch1);
curl_close($ch2);
PHP

); ?>
</div>
</div>
<div class="php8-compare__arrow"></div>
<div class="php8-compare__block example-contents" style="display: table;">
<div class="php8-compare__label php8-compare__label_new">PHP 8.5</div>
<div class="php8-code phpcode" style="display: table-cell;">
<?php highlight_php_trimmed(
<<<'PHP'
$sh = curl_share_init_persistent([
CURL_LOCK_DATA_DNS,
CURL_LOCK_DATA_CONNECT
]);
$ch1 = curl_init('https://php.net/');
curl_setopt($ch1, CURLOPT_SHARE, $sh);
curl_exec($ch1);
$ch2 = curl_init('https://thephp.foundation/');
curl_setopt($ch2, CURLOPT_SHARE, $sh);
curl_exec($ch2);
curl_close($ch1);
curl_close($ch2);
PHP
); ?>
</div>
</div>
</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comparison is not really meaningful to understand what has changed. At least some explanation is necessary. curl_close() should be removed in all cases, since it is useless with 8.0 and deprecated with 8.5.

Comment on lines +240 to +251
#[DataProvider('subtractionProvider')]
public function testSubtraction(
int $minuend,
int $subtrahend,
int $result
): void
{
$this->assertSame(
$result,
Calculator::subtract($minuend, $subtrahend)
);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be kept in sync with the 8.5 version. Unfortunately the 8.5 version can't yet use PHPUnit, since sebastianbergmann/phpunit#6136 is not implemented yet. So both should use some “generic test framework”.

/cc @sebastianbergmann

@TimWolla TimWolla mentioned this pull request Oct 5, 2025
1 task
Comment on lines 189 to 190
|> (fn (string $string) => str_replace(' ', '-', $string))
|> (fn (string $string) => str_replace(['.', '/', '…'], '', $string))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
|> (fn (string $string) => str_replace(' ', '-', $string))
|> (fn (string $string) => str_replace(['.', '/', ''], '', $string))
|> (fn($string) => str_replace(' ', '-', $string))
|> (fn($string) => str_replace(['.', '/', ''], '', $string))

No space after fn for PER-CS. Removed the type, because it seems unlikely for folks to add an explict type in pipes, which will also shorten the line.

/cc @Crell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants