Skip to content
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

Merged
merged 1 commit into from Jul 8, 2022
Merged

Show notification if PHP version is EOL #19421

merged 1 commit into from Jul 8, 2022

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Jun 28, 2022

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

@sgiehl sgiehl added the Needs Review PRs that need a code review label Jun 28, 2022
@sgiehl sgiehl added this to the 4.11.0 milestone Jun 28, 2022
Copy link
Member

@tsteur tsteur left a 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
Copy link
Member

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 👍

@tsteur
Copy link
Member

tsteur commented Jun 29, 2022

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.

Copy link
Contributor

@bx80 bx80 left a 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 👍

@sgiehl sgiehl merged commit 49adc21 into next_release Jul 8, 2022
@sgiehl sgiehl deleted the phpeol branch July 8, 2022 09:08
@basos9
Copy link

basos9 commented Sep 21, 2022

Hello this can cause issues for package maintained releases.
For example php7.3 is supported and receives security fixes in debian buster
https://packages.debian.org/buster/php7.3

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.

@justinvelluppillai justinvelluppillai added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants