WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 3 years ago

Last modified 3 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 3 years ago.
16750.2.diff (24.0 KB) - added by DrewAPicture 3 years ago.
Change 'Optional' style to standard use
16750.3.diff (12.2 KB) - added by ericlewis 3 years ago.
16750.4.diff (13.2 KB) - added by DrewAPicture 3 years ago.

Download all attachments as: .zip

Change History (24)

@CharlesClarkson
5 years ago

Changes for Inline documentation in the l10n.php file.

#1 @scribu
5 years ago

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

#2 follow-up: @hakre
5 years ago

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

#3 in reply to: ↑ 2 ; follow-up: @CharlesClarkson
5 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?

#4 in reply to: ↑ 3 @nacin
5 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.

#5 @SergeyBiryukov
5 years ago

  • Keywords has-patch added

#6 @ericlewis
3 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.

#7 @nacin
3 years ago

  • Milestone changed from Awaiting Review to 3.7

@DrewAPicture
3 years ago

#8 @DrewAPicture
3 years ago

  • Keywords needs-refresh removed

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

#9 @DrewAPicture
3 years ago

  • Keywords commit added

@DrewAPicture
3 years ago

Change 'Optional' style to standard use

#10 @DrewAPicture
3 years ago

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

#11 @SergeyBiryukov
3 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.

#12 @DrewAPicture
3 years ago

In 25492:

Add missing $path parameter phpdoc for load_theme_textdomain().

see #16750.

@ericlewis
3 years ago

#13 @ericlewis
3 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.

#14 follow-up: @SergeyBiryukov
3 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.

#15 in reply to: ↑ 14 @ericlewis
3 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 :)

#16 @DrewAPicture
3 years ago

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

#17 @SergeyBiryukov
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@DrewAPicture
3 years ago

#18 @DrewAPicture
3 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.

#19 @DrewAPicture
3 years ago

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