-
-
Notifications
You must be signed in to change notification settings - Fork 83
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 issues identified by the Theme Check plugin #844
Comments
Hey gang! I ran Theme Check and started going through what looked like 'low hanging fruit.' Would it be helpful to add the report here as a punch list that could be checked off as items get addressed? Not many are a big deal, but there are a couple that are considered to be potential security holes. |
Yes, that would be very helpful. If there are security holes, please open separate issues for those. |
Required items (clear 'violations' of WP coding standards)
Line 144:
Warning items - most are translation issues, but the non-translation items are listed firstnon-translation
Translation functions
Recommended itemsnon-translation
translation
|
It's enormous, so if it would be easier on the eyes I can remove the 'recommended' items for now :) |
…v. 4.5 and higher as listed in issue WPBuddy#844
…v. 4.5 and higher -- the right way -- as listed in issue WPBuddy#844
…-for-title-tag Per issue #844 - adds theme support for <title> , as reported by Theme Check
This is okay; if ( $wp_version < 4.5 ) {
get_currentuserinfo();
} else {
wp_get_current_user();
} |
Translation functions can copy a lot of fixes from #1458, but that was against |
This should be split into separate tickets before work starts on this issue. The changelog for the release containing this should credit the people who have contributed to this issue. |
Next steps here:
|
Research here should also run the tests in #1651 |
Theme check plugin results: Separate issues incoming Warnings
Required
Recommended
Info
|
Focusing on the required items for now: Required
|
We'd need to do a pretty extensive separation of plugin content from Largo, which would take a lot of time. If we wanted to list Largo on wp.org, though, that would be necessary.
Same as above.
I think this is https://github.com/INN/largo/blob/512da701664b329f2f92244bbe54880a6e146431/header.php#L21-L31 , which we fixed in 0.6. It's fine.
To be tackled in #1438
Issue already exists at #1692 |
This issue is getting unreadably long with no clear entry point; let's instead create a new issue for this for the next release. |
Run the Theme Check plugin against a barebones Largo install to determine which issues need fixed.
The text was updated successfully, but these errors were encountered: