Skip to content

Conversation

marcosmarcolin
Copy link
Contributor

The use of Union Types in https://github.com/php/php-src/blob/master/run-tests.php#L3161 is only supported in PHP >= 8.0.0

I believe the information should be updated, or Union Types should be removed.

@cmb69
Copy link
Member

cmb69 commented Nov 20, 2024

Thank you!

Since this has been introduced via 948b2bc, I think we should apply to PHP-8.4 (no need to change the base branch of the PR, though).

@jorgsowa
Copy link
Contributor

Thank you for this PR. I didn't think about bumping the minimum PHP version for the script.

@cmb69
Copy link
Member

cmb69 commented Nov 21, 2024

We may consider to employ some tooling which verifies that we don't use features we do not want to use at some point.

@marcosmarcolin
Copy link
Contributor Author

Hello @cmb69 @jorgsowa,

I don't see any problem in sending it to the 8.4 branch.

However, IMO, if it were to go to the main branch, I don't see it as a problem either, since if someone tries to run the tests with PHP < 8.0 they will get a fatal error.

About compatibility, it makes sense to use something to detect this early, like PHPCompatibility (then we would have to discuss the use of third-party tools, etc.).

@cmb69 cmb69 closed this in 7417c63 Nov 25, 2024
@cmb69
Copy link
Member

cmb69 commented Nov 25, 2024

However, IMO, if it were to go to the main branch, I don't see it as a problem either, since if someone tries to run the tests with PHP < 8.0 they will get a fatal error.

Right, but this comment is not a guard anyway, but rather some documentation. Thus I've applied to PHP-8.4 and merged up.

About compatibility, it makes sense to use something to detect this early, like PHPCompatibility (then we would have to discuss the use of third-party tools, etc.).

Indeed, some discussion would be needed, but at least we have now a policy on using third-party code, so that might not be a problem.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants