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

Add PHP version check for plugin. #174

Merged
merged 14 commits into from
Oct 12, 2023
Merged

Add PHP version check for plugin. #174

merged 14 commits into from
Oct 12, 2023

Conversation

rahulsprajapati
Copy link
Contributor

Description of the Change

Add minimum version check for the plugin before loading it. This will ensure plugin update doesn't break the site that don't match our minimum PHP version.

image

Closes #172

How to test the Change

  1. Setup a WordPress environment running PHP 7.3
  2. Install Block for Apple Maps plugin by downloading it with this feature branch.
  3. Verify that plugin functionality doesn't load and you see an admin error message for php version. ( refer above screenshot in Description )

Changelog Entry

Fixed - Better error handling for environments that don't match our minimum PHP version

Credits

Props @dkotter, @rahulsprajapati

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@rahulsprajapati rahulsprajapati requested review from a team and fabiankaegy as code owners July 20, 2023 12:18
@rahulsprajapati rahulsprajapati requested review from dkotter and removed request for a team July 20, 2023 12:18
@rahulsprajapati rahulsprajapati self-assigned this Jul 20, 2023
- Activation is checking 5.8 as min version, but as per plugin header
  php 7.4 is min requirement.
@jeffpaul jeffpaul requested a review from dkotter September 1, 2023 15:03
@jeffpaul jeffpaul added this to the 1.2.0 milestone Sep 1, 2023
@jeffpaul
Copy link
Member

@ravinderk as you're able during your OSS time, if you could please help handle the code review items here that would be great... thanks!

@ravinderk ravinderk force-pushed the fix/172-php-ver-check branch from c6aaa26 to bbb3fef Compare October 11, 2023 14:33
@ravinderk
Copy link
Contributor

@ravinderk as you're able during your OSS time, if you could please help handle the code review items here that would be great... thanks!

@jeffpaul I updated pull request. I found that E2E test are falling.
@dkotter Do you have any suggestion?

@dkotter
Copy link
Collaborator

dkotter commented Oct 11, 2023

@dkotter Do you have any suggestion?

We need to add composer install --no-dev to our cypress.yml file, otherwise our dependency check fails and the settings never get added (and thus we get a 403 error in our tests when trying to access that settings page)

@ravinderk
Copy link
Contributor

@dkotter Do you have any suggestion?

We need to add composer install --no-dev to our cypress.yml file, otherwise our dependency check fails and the settings never get added (and thus we get a 403 error in our tests when trying to access that settings page)

@dkotter I made the change to build-release-zip.yml in this branch, but cypress.yml is using build-release-zip.yml@develop to create a build for testing. I temporarily replaced the build workflow with build-release-zip.yml@fix/172-php-ver-check to validate E2E tests. I will revert back to the changes before merging the pull request.

@ravinderk ravinderk requested a review from dkotter October 11, 2023 16:08
dkotter
dkotter previously approved these changes Oct 11, 2023
@ravinderk ravinderk merged commit ed79520 into develop Oct 12, 2023
@ravinderk ravinderk deleted the fix/172-php-ver-check branch October 12, 2023 04:54
@dkotter dkotter modified the milestones: 1.2.0, 1.1.2 Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add PHP checks
4 participants