WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#25645 closed defect (bug) (fixed)

Document return value from wp_get_sites()

Reported by: rmccue Owned by: DrewAPicture
Milestone: 3.7 Priority: normal
Severity: normal Version:
Component: Inline Docs Keywords: has-patch
Focuses: Cc:

Description

The new wp_get_sites() function in 3.7 is pretty non-specific on what it actually returns, just noting that it returns "an array of site data" without saying what it actually returns. I really hate functions that do this, as it means you have to use it and dump the result, or check the Codex for an example.

Attachments (3)

25645.diff (2.7 KB) - added by DrewAPicture 8 years ago.
25645.2.diff (2.7 KB) - added by DrewAPicture 8 years ago.
25645.3.diff (2.8 KB) - added by DrewAPicture 8 years ago.

Download all attachments as: .zip

Change History (13)

@DrewAPicture
8 years ago

#1 @DrewAPicture
8 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Awaiting Review to 3.7

@DrewAPicture
8 years ago

#2 @DrewAPicture
8 years ago

I think 25645.2.diff should suffice for now.

See the IRC log for some chatter with @rmccue on the subject.

#3 follow-up: @SergeyBiryukov
8 years ago

I tend to agree with rmccue that listing the exact returned keys (or adding a @see reference if they're already listed somewhere else) would be helpful here.

Last edited 8 years ago by SergeyBiryukov (previous) (diff)

#4 in reply to: ↑ 3 ; follow-up: @nacin
8 years ago

Replying to SergeyBiryukov:

I tend to agree with rmccue that listing the exact returned keys (or adding a @see reference if they're already listed somewhere else) would be helpful here.

It looks like 25645.2.diff does this, but would it make more sense as part of the @return block?

#5 in reply to: ↑ 4 ; follow-up: @DrewAPicture
8 years ago

Replying to nacin:

Replying to SergeyBiryukov:

I tend to agree with rmccue that listing the exact returned keys (or adding a @see reference if they're already listed somewhere else) would be helpful here.

It looks like 25645.2.diff does this, but would it make more sense as part of the @return block?

Fully documenting the @return here has wide implications, as it would set a precedent for documenting returned arrays with hash notations. As far as I'm aware, this would be the first and only instance of such in core.

I'm all for documenting all the things if it means reduced confusion, but I'm also all for consistency. If we do a hash notation here, the standard will need to be updated and we'll need to (eventually) do it everywhere an array is returned. Is that something we want to commit to?

#6 @jeremyfelt
8 years ago

Is there a way to document "@see the DB schema"? It's possible that things would change in the future, but for for the moment each of the results returned is from the blogs table.

#7 in reply to: ↑ 5 ; follow-up: @SergeyBiryukov
8 years ago

Replying to DrewAPicture:

I'm all for documenting all the things if it means reduced confusion, but I'm also all for consistency.

I see, makes sense. Let's go with 25645.2.diff then.

#8 in reply to: ↑ 7 @DrewAPicture
8 years ago

Replying to SergeyBiryukov:

Replying to DrewAPicture:

I'm all for documenting all the things if it means reduced confusion, but I'm also all for consistency.

I see, makes sense. Let's go with 25645.2.diff then.

I'm certainly willing to entertain the idea, but consistency is key. I think doing hash notations on returns would probably give us a more complete result for parsed functional docs (also closer to what we have in the Codex), I just wanted to lay out the implications of moving in that direction :)

Let's do 25645.2.diff for now and try to come up with a longer-term strategy moving forward in 3.8 and beyond.

#9 @nofearinc
8 years ago

+1 for 25645.2.diff as well.

@DrewAPicture
8 years ago

#10 @DrewAPicture
8 years ago

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

In 25862:

Improve inline documentation for the wp_get_sites() return value.

Fixes #25645.

Note: See TracTickets for help on using tickets.