Problem/Motivation
If the module is installed on an old version of Drupal, which uses PHP 5.6, it will break because of return typing in methods
Steps to reproduce
Install the module on an old Drupal with PHP 5.6
Proposed resolution
Change required PHP version in composer.json to 7.1 to match codebase.
Remaining tasks
none
User interface changes
none
API changes
none
Data model changes
none
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | change-composer-php-requirement-to-7.0.patch | 772 bytes | jplanginier |
| change-composer-php-requirement.patch | 301 bytes | jplanginier |
Comments
Comment #2
damienmckennaGood catch. The codebase should be compatible with Drupal 8.9, which requires PHP 7.0, thus it should work with PHP 7.0, rather than 7.1.
Comment #3
jplanginier commentedGiven the class MetatagTokenTest has a return type void, it would still break with a 7.0 compatibility (though only during tests, obviously).
Here is the patch to allow the module to be 7.0 compliant then
Comment #4
jplanginier commentedHere is the changes for PHP 7.0, given there was a return type void, which is not allowed in PHP 7.0 !
Comment #5
damienmckennaThe only instance of 'void' is in a test, which won't work anyway because of changes to PHPUnit. I did some manual testing with PHP 7.0 on a local ddev install and I couldn't trigger any errors.
Is there any way to indicate in composer.json that PHP 7.1 is needed to run the tests, e.g. can "php" be set to "^7.1" in require-dev?
Comment #6
damienmckennaFYI I published your patch and confirmed your account.
Comment #7
jplanginier commentedThank you.
I checked back on composer and after some tests, it looks like if we declare another version in the "require-dev" key, the highest will be used, no matter whether --no-dev agument is used.
So we either change the test to be 7.0 compliant (which doesn't affect anything) or we set the module to 7.1 (which will prevent some people from using it on their 7.0 install).
I think the changes you suggested are the right option.
Comment #8
damienmckennaI think just removing the change for the test will be enough, which I'll do when I commit the change; Drupal 8 & 9 require PHP 7.0 already and Metatag shouldn't increase that requirement, so I don't think we need to modify the composer.json file.
Comment #9
damienmckennaCommitted. Thank you.