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

[Wordpress] Plugin & Theme required php #5723

Merged
merged 6 commits into from
Oct 17, 2020
Merged

[Wordpress] Plugin & Theme required php #5723

merged 6 commits into from
Oct 17, 2020

Conversation

JoeIzzard
Copy link
Contributor

Related to #5493, this adds the required php badge for both plugin and themes.

One difference between this and the main PR is that I have now changed one of the tests to a mock, as the live data I was using has changed and no longer works for the test, and I am struggling to find any live data to replace it with.

To help with this, I have refactored some of the test code to help facilitate this.

Fixed existing testing to conform to new api options and schema. Also added testing for the new badge
Changed the Theme Not Set test to a mock as unable to find a live example (previously used 'generatepress' however this now has the data). Also split out query selector fields to allow updating in one place for themes and plugins
@shields-ci
Copy link

shields-ci commented Oct 16, 2020

Messages
📖 ✨ Thanks for your contribution to Shields, @JoeIzzard!

Generated by 🚫 dangerJS against e40ad4e

@shields-cd shields-cd temporarily deployed to shields-staging-pr-5723 October 16, 2020 19:11 Inactive
Copy link
Member

@chris48s chris48s left a comment

Choose a reason for hiding this comment

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

Nice. Thanks so much for the extra work on splitting these up. Might take a bit to get through them all, but now each review is much less daunting :)

If requires_php is '7.2' or '5.6' or whatever, is that a minimum version i.e: is what this really means the plugin/theme is compatible with PHP >=7.2 or >=5.6?
If so, how about we style it like - it makes the meaning clearer and will be more consistent with other PHP version badges which express the PHP version requirement as a constraint.

@JoeIzzard
Copy link
Contributor Author

No problem, at least now they are easy to do a quick review of for you guys, so should be quicker than trying to do it all as one!

Yeah that's right. That's definitely a clearer way to style it, I will get that changed.

@shields-cd shields-cd temporarily deployed to shields-staging-pr-5723 October 16, 2020 23:36 Inactive
@calebcartwright calebcartwright added the service-badge New or updated service badge label Oct 17, 2020
@github-actions
Copy link
Contributor

This pull request was merged to master branch. This change is now waiting for deployment, which will usually happen within a few days. Stay tuned by joining our #ops channel on Discord!

After deployment, changes are copied to gh-pages branch:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge New or updated service badge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants