-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Docs: Add missing @return descriptions in bundled themes. #10533
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
Docs: Add missing @return descriptions in bundled themes. #10533
Conversation
Adds missing descriptions to tags and adds missing tags in various files across Twenty Fourteen, Twenty Seventeen, Twenty Nineteen, Twenty Twenty, and Twenty Twenty-One to comply with WordPress PHP Documentation Standards.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
| * @since Twenty Fourteen 3.6 Replaced Google URL with self-hosted fonts. | ||
| * | ||
| * @return string | ||
| * @return string Fonts URL for the theme. |
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.
Good catch that the description was missing in Twenty Fourteen, but I think the description in twentytwelve_get_font_url() and twentythirteen_fonts_url() would fit better.
| * @return string Fonts URL for the theme. | |
| * @return string Font stylesheet or empty string if disabled. |
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.
How about “Font stylesheet URL”?
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.
Then I would want "Font stylesheet URL" for T12, T13 and T14 :)
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.
I noticed that Twenty Fourteen, Twenty Fifteen, Twenty Sixteen, and Twenty Seventeen all use the @return string Fonts URL for the theme. description. Should I update: @return string Font stylesheet or empty string if disabled. for consistency?
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.
Maybe it should be the same for all six themes from Twenty Twelve to Twenty Seventeen:
Font stylesheet URL or empty string if disabled.
I wanted to say something like "if all fonts are disabled" for Twenty Fifteen and Twenty Sixteen because locales can turn off fonts individually, but the same would fit for Twenty Thirteen and that was not any different than the themes with only one hosted font.
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.
Thanks for the clarification! I agree using “Font stylesheet URL or empty string if disabled” consistently across Twenty Twelve to Twenty Seventeen makes sense. I’ll update all six themes accordingly.
src/wp-content/themes/twentynineteen/inc/template-functions.php
Outdated
Show resolved
Hide resolved
src/wp-content/themes/twentyseventeen/inc/template-functions.php
Outdated
Show resolved
Hide resolved
src/wp-content/themes/twentytwentyone/inc/template-functions.php
Outdated
Show resolved
Hide resolved
src/wp-content/themes/twentytwentyone/inc/template-functions.php
Outdated
Show resolved
Hide resolved
| * @return string | ||
| * @return string CSS styles for non-Latin languages based on the site locale. | ||
| */ | ||
| function twenty_twenty_one_get_non_latin_css( $type = 'front-end' ) { |
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.
This function is calling twenty_twenty_one_generate_css() which also lacks a @return description.
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.
Okay, I will update it. Thanks for pointing that out!
Co-authored-by: Weston Ruter <westonruter@gmail.com>
| * @since 6.0.0 | ||
| * | ||
| * @return string List item separator. |
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.
- The core
wp_get_list_item_separator()function describes the return value as "Locale-specific list item separator." I think the same is accurate within the themes, though these only have the fallback translatable string instead of using the PHP class. - Five other themes also include the function for backward compatibility: T11, T12, T13, T17, and T21. See r52929.
- I think the
sinceannotations should reflect the theme versions, not the WordPress version (as r45256 did forwp_body_open()"shims").- Twenty Eleven 4.1
- Twenty Twelve 3.7
- Twenty Thirteen 3.7
- Twenty Seventeen 3.0
- Twenty Nineteen 2.3
- Twenty Twenty-One 1.6
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.
Okay, I will update it. Thanks for pointing that out!
…y and Twenty Twenty-One
…urn` description for `wp_get_list_item_separator()` in various themes.
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Developed in #10533 Props huzaifaalmesbah, sabernhardt, palak678, westonruter. See #64224. Fixes #64277. git-svn-id: https://develop.svn.wordpress.org/trunk@61302 602fd350-edb4-49c9-b593-d223f7449a82
Developed in WordPress/wordpress-develop#10533 Props huzaifaalmesbah, sabernhardt, palak678, westonruter. See #64224. Fixes #64277. Built from https://develop.svn.wordpress.org/trunk@61302 git-svn-id: http://core.svn.wordpress.org/trunk@60614 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Developed in WordPress/wordpress-develop#10533 Props huzaifaalmesbah, sabernhardt, palak678, westonruter. See #64224. Fixes #64277. Built from https://develop.svn.wordpress.org/trunk@61302 git-svn-id: https://core.svn.wordpress.org/trunk@60614 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This PR addresses missing or incomplete
@returntag documentation in several bundled themes.Changes include:
@returntags infunctions.phpand other include files for Twenty Twenty, Twenty Seventeen, and Twenty Twenty-One.@returntags and descriptions in Twenty Fourteen and Twenty Nineteen.Trac ticket: https://core.trac.wordpress.org/ticket/64277