Skip to content

Bump minimum PHP version from 5.6 to 7.0 #7088

Merged
westonruter merged 10 commits into
developfrom
enhancement/bump-php-version
May 9, 2022
Merged

Bump minimum PHP version from 5.6 to 7.0 #7088
westonruter merged 10 commits into
developfrom
enhancement/bump-php-version

Conversation

@dhaval-parekh
Copy link
Copy Markdown
Collaborator

@dhaval-parekh dhaval-parekh commented Apr 28, 2022 •
edited
Loading

Summary

Fixes #7031

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

westonruter
westonruter reviewed Apr 28, 2022
View reviewed changes
Comment thread composer.json
@dhaval-parekh dhaval-parekh force-pushed the enhancement/bump-php-version branch from dc13e04 to e4bc052 Compare May 3, 2022 07:19
@dhaval-parekh
Copy link
Copy Markdown
Collaborator Author

dhaval-parekh commented May 3, 2022

Currently, The GH action is failing for PHP 8.0 because of the PHPUnit package. It uses the match keyword which is reserved in PHP 8.0, However that is fixed in the 9.3 version of PHPUnit but that requires PHP 7.3 or higher.

Finding a workaround to fix this.

@dhaval-parekh dhaval-parekh force-pushed the enhancement/bump-php-version branch from fd4452c to b5025b4 Compare May 4, 2022 11:51
@dhaval-parekh dhaval-parekh marked this pull request as ready for review May 5, 2022 07:11
@dhaval-parekh dhaval-parekh requested a review from westonruter May 5, 2022 07:11
@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2022 •
edited
Loading

Codecov Report

Merging #7088 (26e322e) into develop (7c6018d) will increase coverage by 13.07%.
The diff coverage is 0.00%.

❗ Current head 26e322e differs from pull request most recent head c638959. Consider uploading reports for the commit c638959 to get more accurate results

Impacted file tree graph

@@              Coverage Diff               @@
##             develop    #7088       +/-   ##
==============================================
+ Coverage      64.88%   77.95%   +13.07%     
- Complexity         0     7097     +7097     
==============================================
  Files             67      273      +206     
  Lines           1159    21991    +20832     
==============================================
+ Hits             752    17143    +16391     
- Misses           407     4848     +4441
Flag Coverage Δ
javascript 64.88% <ø> (ø)
php 78.68% <0.00%> (?)
unit 78.68% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
amp.php 0.00% <0.00%> (ø)
includes/embeds/class-amp-gfycat-embed-handler.php 75.00% <0.00%> (ø)
...ucture/ServiceContainer/SimpleServiceContainer.php 100.00% <0.00%> (ø)
includes/widgets/class-amp-widget-archives.php 0.00% <0.00%> (ø)
src/Exception/InvalidEventProperties.php 95.23% <0.00%> (ø)
src/Services.php 75.00% <0.00%> (ø)
src/LoadingError.php 100.00% <0.00%> (ø)
...udes/embeds/class-amp-soundcloud-embed-handler.php 75.00% <0.00%> (ø)
src/RemoteRequest/WpHttpRemoteGetRequest.php 60.52% <0.00%> (ø)
includes/embeds/class-amp-base-embed-handler.php 45.45% <0.00%> (ø)
... and 197 more

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2022 •
edited
Loading

Plugin builds for e0ac132 are ready 🛎️!

@westonruter westonruter added this to the v2.3 milestone May 5, 2022
westonruter
westonruter reviewed May 5, 2022
View reviewed changes
Comment thread composer.json
"phpunit/phpunit-mock-objects": {
"Fix ReflectionParameter warnings on PHP 8": "patches/phpunit-mock-objects.patch"
"Fix ReflectionParameter warnings on PHP 8": "patches/phpunit-mock-objects.patch",
"Remove 'match' keyword from uses <https://github.com/sebastianbergmann/phpunit/issues/4373>": "patches/remove-match-keyword.patch"
Copy link
Copy Markdown
Member

@westonruter westonruter May 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding #7088 (comment), I'm curious how you were able to find the solution.

Copy link
Copy Markdown
Collaborator Author

@dhaval-parekh dhaval-parekh May 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added in 613eca commit. And for that, I took some references from here.

So it basically applies changes of this PR into an installed version of phpunit/phpunit-mock-objects package.

westonruter
westonruter reviewed May 5, 2022
View reviewed changes
Comment thread composer.json
"patches": {
"phpunit/phpunit-mock-objects": {
"Fix ReflectionParameter warnings on PHP 8": "patches/phpunit-mock-objects.patch"
"Fix ReflectionParameter warnings on PHP 8": "patches/phpunit-mock-objects.patch",
Copy link
Copy Markdown
Member

@westonruter westonruter May 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this patch just need to be refreshed?

Copy link
Copy Markdown
Member

@westonruter westonruter May 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or rather, I'm curious how you came up with this change: 4725407

Copy link
Copy Markdown
Collaborator Author

@dhaval-parekh dhaval-parekh May 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, We need to update that patch. Because previously it was used the ^3.2 version. and now it's using the ^5.0.9 version. But after 3.4.4 directory structure is changed ( see 4.0.0 ) So therefor we need to update patch file.

westonruter
westonruter reviewed May 5, 2022
View reviewed changes
Comment thread composer.lock
"This file is @generated automatically"
],
"content-hash": "5e242b23d84049cea21e9551fed88cc1",
"content-hash": "7fd6292577e257a09b6b3155ad9f771b",
Copy link
Copy Markdown
Member

@westonruter westonruter May 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious how you updated the composer.lock. Did you just remove it and then run composer install?

Copy link
Copy Markdown
Collaborator Author

@dhaval-parekh dhaval-parekh May 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I removed vendors and composer.lock and then install the dependency.

@westonruter westonruter changed the title Bump PHP version from 5.6 to 7.0 Bump minimum PHP version from 5.6 to 7.0 May 5, 2022
westonruter
westonruter approved these changes May 5, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good. Just would like to learn more re: the comments I left prior to merging.

@westonruter westonruter merged commit aa15c91 into develop May 9, 2022
@westonruter westonruter deleted the enhancement/bump-php-version branch May 9, 2022 16:28
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Jun 14, 2022
@westonruter westonruter mentioned this pull request Sep 22, 2022
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@westonruter westonruter westonruter approved these changes

Assignees

No one assigned

Labels

Changelogged Whether the issue/PR has been added to release notes.

Projects

None yet

Milestone

v2.3

Development

Successfully merging this pull request may close these issues.

Bump minimum PHP version from 5.6 to 7.0

2 participants

@dhaval-parekh @westonruter