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.

Sign up for GitHub

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

Jump to bottom

Bump minimum PHP version from 5.6 to 7.0 #7088

Merged
merged 10 commits into from
May 9, 2022
Merged

Bump minimum PHP version from 5.6 to 7.0 #7088

merged 10 commits into from
May 9, 2022

Conversation

dhaval-parekh
Copy link
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).

@dhaval-parekh
Copy link
Collaborator Author

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 marked this pull request as ready for review May 5, 2022 07:11
@codecov
Copy link

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
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
composer.json
@@ -90,7 +90,8 @@
},
"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",
"Remove 'match' keyword from uses <https://github.com/sebastianbergmann/phpunit/issues/4373>": "patches/remove-match-keyword.patch"
Copy link
Member

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
Collaborator Author

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
composer.json
"Fix ReflectionParameter warnings on PHP 8": "patches/phpunit-mock-objects.patch"
"Fix ReflectionParameter warnings on PHP 8": "patches/phpunit-mock-objects.patch",
Copy link
Member

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
Member

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
Collaborator Author

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
composer.lock
@@ -4,7 +4,7 @@
"Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
"This file is @generated automatically"
],
"content-hash": "5e242b23d84049cea21e9551fed88cc1",
"content-hash": "7fd6292577e257a09b6b3155ad9f771b",
Copy link
Member

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
Collaborator Author

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