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

Comments

jplanginier created an issue. See original summary.

damienmckenna’s picture

Issue summary: View changes

Good 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.

jplanginier’s picture

StatusFileSize
new772 bytes

Given 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

jplanginier’s picture

Here is the changes for PHP 7.0, given there was a return type void, which is not allowed in PHP 7.0 !

damienmckenna’s picture

The 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?

damienmckenna’s picture

Issue summary: View changes
Status: Active » Needs review

FYI I published your patch and confirmed your account.

jplanginier’s picture

Thank 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.

damienmckenna’s picture

Assigned: jplanginier » Unassigned
Status: Needs review » Reviewed & tested by the community
Parent issue: » #3203686: Plan for Metatag 8.x-1.17

I 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.

damienmckenna’s picture

Status: Reviewed & tested by the community » Fixed

Committed. Thank you.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.