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

PHP8 compatibility #150

Merged
merged 17 commits into from
Mar 4, 2022
Merged

PHP8 compatibility #150

merged 17 commits into from
Mar 4, 2022

Conversation

cadic
Copy link
Contributor

@cadic cadic commented Feb 18, 2022

Description of the Change

Added tests for compatibility against PHP7 and PHP8:

  • PHPCompatibilityWP 7.0, 7.4, 8.0, 8.1
  • PHPUnit against 7.4, 8.1

Updated PHPUnit to 9.5, updated unit tests to be compatible with 9.5

Verification Process

Check GitHub Actions workflows all pass.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Changelog Entry

Added - Compatibility tests against PHP 7 and 8.
Changed - Unit tests against PHP 8
Changed - Bump required PHP 7.0

@jeffpaul jeffpaul added this to the Future Release milestone Feb 21, 2022
@cadic cadic requested a review from dkotter February 28, 2022 09:10
@cadic cadic marked this pull request as ready for review February 28, 2022 09:16
readme.txt Outdated
@@ -3,7 +3,7 @@ Contributors: 10up, helen, adamsilverstein, jakemgold
Tags: simple podcasting, podcasting, podcast, apple podcasts, episode, gutenberg, blocks, block
Requires at least: 4.6
Tested up to: 5.9
Requires PHP: 5.3
Requires PHP: 7.3
Copy link
Collaborator

Choose a reason for hiding this comment

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

Curious why we're bumping from 5.3 all the way to 7.3? Doesn't seem like we have any new features we're adding that require 7.3 so just wondering on the reason here? I think for sure we should drop support for 5.3 at this point but maybe we just bump to 7.0? We also need to update the README.md file as well, as that mentions 5.3 as the minimum supported version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, we should keep 7.0 support. Updated both readme files and added 7.0 to compatibility testing.

The initial bump 7.3 was made together with composer dependency (PHPUnit 9.5 requires at least PHP 7.3). This is dev-only requirement and of course the plugin itself works well with 7.0.

@dkotter
Copy link
Collaborator

dkotter commented Mar 2, 2022

There's some merge conflicts here that need fixing

@cadic
Copy link
Contributor Author

cadic commented Mar 3, 2022

@dkotter resolved conflicts and fixed PHP minimum reqs in readme files.

I also reduced the number of Unit tests just to cover latest PHP 7 and 8, not sure it makes sense to test against every release within a major version. We still test compatibility against 7.0, 7.4, 8.0 and 8.1 in separate workflow

@cadic cadic requested a review from dkotter March 3, 2022 06:48
@cadic cadic merged commit da4578e into develop Mar 4, 2022
@cadic cadic deleted the tests/php8-compat branch March 4, 2022 16:45
@cadic cadic modified the milestones: Future Release, 1.3.0 Mar 4, 2022
@cadic cadic modified the milestones: 1.3.0, 1.2.3 Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants