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

Fix/154 PHP Version Check #165

Merged
merged 8 commits into from
Dec 14, 2023
Merged

Fix/154 PHP Version Check #165

merged 8 commits into from
Dec 14, 2023

Conversation

bmarshall511
Copy link
Contributor

Description of the Change

Added a minimum PHP version check for 7.4.

Closes #154

How to test the Change

  1. Spin up the site with a version of PHP below 7.4
  2. Verify an admin notice has been added alerting the user of the failed check

Changelog Entry

Added - >=PHP 7.4 version check

Credits

Props @bmarshall511

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.

@bmarshall511 bmarshall511 added the needs:code-review This requires code review. label Sep 18, 2023
@bmarshall511 bmarshall511 self-assigned this Sep 18, 2023
@github-actions github-actions bot added this to the Future Release milestone Sep 18, 2023
@jeffpaul jeffpaul modified the milestones: Future Release, 1.3.3 Sep 18, 2023
Copy link
Member

@faisal-alvi faisal-alvi left a comment

Choose a reason for hiding this comment

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

@bmarshall511 Thank you for the PR, LGTM. I tested it and saw the notice when the PHP version is below the minimum required. However, outlined a few minor tweaks below.

publisher-media-kit.php Outdated Show resolved Hide resolved
publisher-media-kit.php Show resolved Hide resolved
publisher-media-kit.php Outdated Show resolved Hide resolved
publisher-media-kit.php Outdated Show resolved Hide resolved
publisher-media-kit.php Outdated Show resolved Hide resolved
publisher-media-kit.php Show resolved Hide resolved
@bmarshall511
Copy link
Contributor Author

@bmarshall511 Thank you for the PR, LGTM. I tested it and saw the notice when the PHP version is below the minimum required. However, outlined a few minor tweaks below.

Thanks for taking a look @faisal-alvi! Updates made, ready for your review again.

faisal-alvi
faisal-alvi previously approved these changes Oct 2, 2023
Copy link
Member

@faisal-alvi faisal-alvi left a comment

Choose a reason for hiding this comment

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

@bmarshall511 The cypress tests are failing, I re ran the failed tests but they are still failing. I can see they passed in your first commit, but failed in the latest, can you please check it?

https://github.com/10up/publisher-media-kit/actions/runs/6352469169/job/17304966900#step:9:164

@faisal-alvi faisal-alvi modified the milestones: 1.3.3, 1.3.4 Oct 11, 2023
@faisal-alvi
Copy link
Member

Note: The failed cypress tests are supposed to be fixed in #166

Let's wait until that is merged.

@ankitguptaindia
Copy link
Member

Test Report: ✅

PR works as expected.

When Plugin is already activated and PHP is downgraded later:

Screenshot 2023-10-30 at 8 07 12 PM

When activating the plugin on PHP below 7.4:

Screenshot 2023-10-30 at 8 08 49 PM

Copy link
Member

@faisal-alvi faisal-alvi left a comment

Choose a reason for hiding this comment

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

Manually updated the minimum required PHP version in the code to 8.2 to test in the 8.1 environment and it worked fine.

image

@faisal-alvi
Copy link
Member

@jeffpaul can you please help to merge this one?

@jeffpaul jeffpaul merged commit 02dabb9 into develop Dec 14, 2023
@jeffpaul jeffpaul deleted the fix/154-php-version-check branch December 14, 2023 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs:code-review This requires code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add PHP checks
4 participants