-
Notifications
You must be signed in to change notification settings - Fork 113
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
Try: Add simplified readme validator #376
base: master
Are you sure you want to change the base?
Conversation
Replace short array syntax Remove the readme.txt file from class-file-check.php Update the error message when the file is missing.
Thank you. |
Test with a theme that is missing a readme.txt and readme.md Confirm that the error about the missing readme is displaying. Test with a theme with missing headers in the readme.
Confirm that the warnings are displaying. Test with a theme with incorrect headers in the readme. my readme header is like below,
Confirm that the warnings are displaying. On a side note, is this INFO necessary?
|
The requires at least is compared to |
The info does make it noisy. My thought was to: But if you prefer it can be removed and reconsidered once there are solid plans to show it in the directory. |
Yeah I think it is broken. I'd like to compare with the live plugin validator. |
Looks good.
|
protected $latest_wordpress_version = '5.8'; We need to make it dynamic. |
When I added 2 users without “,” comma between them, the check returned no error.
|
I have not used the sniffer for so long, that I forgot that it checks for this: |
But how? |
Thanks for the link. I got the contributor validation working based on the sniffer. It’s silly but I don’t know how to create a branch off a branch. Any suggestions on how to proceed. |
Co-authored-by: Dion Hulse <[email protected]>
@dd32 What will happen if my local WordPress version is less than the current version? |
No notice is shown in this case, But if we go above If the WordPress version will be higher than 5.9, then do we need to reset the version again? Nothing shows for, Rest of the things looks fine to me. |
We would have had to but Dion's update solves that. |
Okay, thanks for the update. Any idea how to fix the readme issues with spacing? |
I am not sure either 😊 Maybe we need to do it in two steps and merge this one first? |
I'll see if I can make it remove blank lines from the header, but the warning is also correct, the blank line must not be there. |
Check if the name in the readme matches the theme name, if not, error. Update the $latest_wordpres_version variable. Remove empty lines to prevent errors with the check.
8e696d2 Checks if the name in the readme matches the theme name, if not, error. To avoid reading the wrong readme file if there is more than one readme, this PR needs to be tested together with |
Does the license and license URI have a single space after the colon? |
Requires at least: 5.5 Tested with both adding space, removing space, double space. The result is same. |
Sync branch with master
I think I found the bug where it does not show that the license is missing, but I need your full readme file to confirm. It is not only checking the file header, so if the words "License: blah blah" are elsewhere in the readme file, like for an asset, it will use that. |
Try to solve timeout issues. Check for uppercase file names as well as lowercase with stripos.
Fixes #375
This check and the accompanying parser is copied from the plugin readme validator.
It is more limited than the plugin readme validator, and it does not work for themes that are in sub folders.
The subfolder issue may be solved by #370. Untested.
How to test: