-
Notifications
You must be signed in to change notification settings - Fork 63
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
Detect missing readme headers #414
Detect missing readme headers #414
Conversation
369ff9f
to
71fcdb0
Compare
e5ca523
to
ecc4a54
Compare
fc806bb
to
9a125df
Compare
Hi @ernilambar thanks for the change. It looks good, if you finish the tests I can continue with the review. |
PR has been rebased with trunk and few more updates have been done. |
For me It's ok after all changes! In the future, we would need to get the actual version of WordPress somewhere and create a warning if it's not getting an actual version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything seems OK!
I would add to check the actual version of WordPress. It must be Tested up "actual version". In order to have always the latest version, I've asked to meta, and we have some methods to check that:
So finally we have a complete check. The error that we show to the users is something like this |
@davidperezgar Should this be done in this same PR or separate PR? |
In a separate Issue. I'll create the issue #481 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
Related to #402
check_headers()
method inPlugin_Readme_Check
get_ignored_warnings()
Tested up to
Contributors
wp_plugin_check_ignored_readme_warnings
contributor_ignored
is returned from the above filter, detecting ofContributors
is disabled.If the PR is acceptable then I will add tests for the changes and update other tests accordingly. Other tests also need to be updated because currently we have assumed there will be no warning for missing
Tested up to
field.