Make WordPress Core

Opened 13 years ago

Closed 11 years ago

Last modified 11 years ago

#16750 closed enhancement (fixed)

Improve inline docs for l10n functions

Reported by: charlesclarkson's profile CharlesClarkson Owned by: sergeybiryukov's profile 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 13 years ago.
Changes for Inline documentation in the l10n.php file.
16750.diff (24.0 KB) - added by DrewAPicture 11 years ago.
16750.2.diff (24.0 KB) - added by DrewAPicture 11 years ago.
Change 'Optional' style to standard use
16750.3.diff (12.2 KB) - added by ericlewis 11 years ago.
16750.4.diff (13.2 KB) - added by DrewAPicture 11 years ago.

Download all attachments as: .zip

Change History (24)

@CharlesClarkson
13 years ago

Changes for Inline documentation in the l10n.php file.

#1 @scribu
13 years ago

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

#2 follow-up: @hakre
13 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
13 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
13 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
13 years ago

  • Keywords has-patch added

#6 @ericlewis
11 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
11 years ago

  • Milestone changed from Awaiting Review to 3.7

@DrewAPicture
11 years ago

#8 @DrewAPicture
11 years ago

  • Keywords needs-refresh removed

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

#9 @DrewAPicture
11 years ago

  • Keywords commit added

@DrewAPicture
11 years ago

Change 'Optional' style to standard use

#10 @DrewAPicture
11 years ago

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

#11 @SergeyBiryukov
11 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
11 years ago

In 25492:

Add missing $path parameter phpdoc for load_theme_textdomain().

see #16750.

@ericlewis
11 years ago

#13 @ericlewis
11 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
11 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
11 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
11 years ago

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

#17 @SergeyBiryukov
11 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#18 @DrewAPicture
11 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
11 years ago

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