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

Replace str_contains() call on moodle branch with PHP 7.4-compatible equivalent #194

Open
Limekiller opened this issue Oct 11, 2024 · 6 comments

Comments

@Limekiller
Copy link

Recently a change was made to the library included in the Moodle plugin mod_hvp that introduces a function call to str_contains(), which is only compatible with PHP 8.0 and higher. However, the plugin claims to support Moodle versions that do not support PHP 8.0, as well as Moodle 4.1, which is still actively supported by Moodle and is fully compatible with PHP 7.4.

Moodle sites on the actively-supported Moodle 4.1 that are running on PHP 7.4, or older sites that do not support PHP 8.0 but are claimed to be supported by mod_hvp, will encounter errors until the function call is removed. I have created a pull request to do so here.

@cli-ish
Copy link

cli-ish commented Oct 21, 2024

I have the same issue while upgrading. Hope for a quick merge.

@otacke
Copy link
Contributor

otacke commented Oct 21, 2024

Please note that while older Moodle versions still support PHP 7, PHP 7 reached its end of life in November 2021 and has not received security updates for almost 2 years. So, while I fully understand why people want this to run (and the change is really marginal, of course), I wonder if indirectly supporting to further delay the migration to PHP 8 would be the right signal.
(not a member of H5P Group and not expressing their opinions)

@relecand
Copy link

relecand commented Oct 21, 2024

Ubuntu (and Debian) continues to provide security updates for PHP 7.4 through its long-term support system, even though official PHP support ended. This LTS ensures that security patches are backported. However, these updates do not include new features or non-essential bug fixes.

https://changelogs.ubuntu.com/changelogs/pool/main/p/php7.4/

@otacke
Copy link
Contributor

otacke commented Oct 21, 2024

That's good for Ubuntu and Debian.

@Limekiller
Copy link
Author

I don't actually have an opinion on if H5P should support PHP 7 or not, but that if the decision is made that it no longer should, the plugin needs to be updated to indicate that it no longer supports older Moodle versions. Currently mod_hvp claims it supports every Moodle version from 2.5; requiring PHP 8.0+ would change that to, at most, 4.0 and up--and I would argue, should really only be 4.2, 4.3, and 4.4.

@otacke
Copy link
Contributor

otacke commented Oct 21, 2024

@Limekiller You're right, I know about that compatibility claim. It's up to H5P Group either way. It's just my personal view that I'd rather encourage people to migrate to PHP 8, and I merely wondered what opinion I might face here.
And one hint: In my experience, H5P Group does hardly ever react to pull requests unless you create awareness by raising an issue at https://h5ptechnology.atlassian.net/jira/software/c/projects/HFP/issues So, if it's important to you ...

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

No branches or pull requests

4 participants