New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Show notification if PHP version is EOL #19421
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With all the other changes in the PR it's a bit hard to say what changed but looks good 👍
Can we also set up some process to make sure we won't forget adding future php versions in the future? We can already merge independently if tests pass.
And if it's just something as simple as that a test will always fail if the date is passed January 1st 2023 and we then add a new php version check and fix the test by changing the date to next year.
return version_compare(PHP_VERSION, '7.1', '>='); | ||
$phpEOL = '7.3'; | ||
|
||
// End of security update for certain PHP versions as of https://www.php.net/supported-versions.php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, that's great and so it works for the next PHP versions right away 👍
I haven't tested it but should be fine to move it in next_release if we're confident it works. There will be still RC for a while anyway. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done some basic functional testing and reviewed the code. I can't see any issues with this, it all looks good 👍
Hello this can cause issues for package maintained releases. This should be made configurable such as users who have an extended supported php version by a package maintainer should not be confused/scared of running EOL software. |
Description:
@tsteur let us know if we shall include that in Matomo 4.11 already. Otherwise I can also move it to 4.12.
Review