WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 2 years ago

Last modified 2 years ago

#16750 closed enhancement (fixed)

Improve inline docs for l10n functions

Reported by: CharlesClarkson Owned by: SergeyBiryukov
Milestone: 3.7 Priority: normal
Severity: trivial Version: 3.1
Component: Inline Docs Keywords: has-patch
Focuses: Cc:

Description

Changes to the inline documentation in WordPress 3.1.

These are documentation changes only.

Attachments (5)

l10n.diff (20.3 KB) - added by CharlesClarkson 5 years ago.
Changes for Inline documentation in the l10n.php file.
16750.diff (24.0 KB) - added by DrewAPicture 2 years ago.
16750.2.diff (24.0 KB) - added by DrewAPicture 2 years ago.
Change 'Optional' style to standard use
16750.3.diff (12.2 KB) - added by ericlewis 2 years ago.
16750.4.diff (13.2 KB) - added by DrewAPicture 2 years ago.

Download all attachments as: .zip

Change History (24)

@CharlesClarkson5 years ago

Changes for Inline documentation in the l10n.php file.

comment:1 @scribu5 years ago

  • Summary changed from Inline Documentation Changes for WP 3.1 to Improve inline docs for l10n functions

comment:2 follow-up: @hakre5 years ago

Thanks for providing the patch. Which wordwrap setting have you used for the text in comments?

comment:3 in reply to: ↑ 2 ; follow-up: @CharlesClarkson5 years ago

Replying to hakre:

Which wordwrap setting have you used for the text in comments?

I wrapped at 80 characters. Except in lines 349 & 496. Is there a standard wrap for WordPress files?

comment:4 in reply to: ↑ 3 @nacin5 years ago

Replying to CharlesClarkson:

Replying to hakre:

Which wordwrap setting have you used for the text in comments?

I wrapped at 80 characters. Except in lines 349 & 496. Is there a standard wrap for WordPress files?

Nope.

comment:5 @SergeyBiryukov4 years ago

  • Keywords has-patch added

comment:6 @ericlewis2 years ago

  • Keywords needs-refresh added

Patch needs refresh. A lot of this is word wrap changes and adding periods at the end of sentences.

Hopefully changes in word wrap and punctuation will get specific direction and instruction from the Inline Docs group early in the 3.7 cycle.

comment:7 @nacin2 years ago

  • Milestone changed from Awaiting Review to 3.7

@DrewAPicture2 years ago

comment:8 @DrewAPicture2 years ago

  • Keywords needs-refresh removed

16750.diff builds off of and refreshes l10n.diff at r25449

comment:9 @DrewAPicture2 years ago

  • Keywords commit added

@DrewAPicture2 years ago

Change 'Optional' style to standard use

comment:10 @DrewAPicture2 years ago

16750.2.diff uses the 'Optional. ' style per the inline docs standards.

comment:11 @SergeyBiryukov2 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 25458:

Update inline documentation for l10n functions. props CharlesClarkson, DrewAPicture. fixes #16750.

comment:12 @DrewAPicture2 years ago

In 25492:

Add missing $path parameter phpdoc for load_theme_textdomain().

see #16750.

@ericlewis2 years ago

comment:13 @ericlewis2 years ago

attachment:16750.3.diff is leftover from some work I did in #24212 that categorically belongs on this ticket.

All references to "domain" are replaced with "text domain" for more explicit helpful description. Descriptions in docblocks for $domain are simplified to "Text domain." rather then "Unique identifier for retrieving translated strings." which is a bit more confusing off the bat.

comment:14 follow-up: @SergeyBiryukov2 years ago

s/domain/text domain/ seems fine.

For the longer string, I'd suggest adding "Text domain" in front of it, rather than replacing it. There should still be some short description, and the "unique identifier" serves that well.

comment:15 in reply to: ↑ 14 @ericlewis2 years ago

Replying to SergeyBiryukov:

s/domain/text domain/ seems fine.

For the longer string, I'd suggest adding "Text domain" in front of it, rather than replacing it. There should still be some short description, and the "unique identifier" serves that well.

Here I thought giving the entire name for the unique identifier (Text domain) is more helpful instead of telling that is a unique identifier. There are a lot of unique identifiers that get tossed around WP core (Post ID, taxonomy name, etc.), and developers can be expected to look up the l18n API to understand what a text domain is, and that a text domain should be a unique identifier, which is why I left it out. It's a grey area between where we expect developers to seek more complete documentation and brevity.

Please tell me if I'm getting too pedantic :)

comment:16 @DrewAPicture2 years ago

  • Keywords needs-patch added; has-patch commit removed

comment:17 @SergeyBiryukov2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@DrewAPicture2 years ago

comment:18 @DrewAPicture2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 25586:

Standardize usage of 'text domain' in inline documentation for wp-includes/l10n.php.

Props ericlewis, SergeyBiryukov.
Fixes #16750.

comment:19 @DrewAPicture2 years ago

  • Keywords has-patch added; needs-patch removed
Note: See TracTickets for help on using tickets.