Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#41619 closed defect (bug) (fixed)

Fix documentation issues for `domain_exists()`

Reported by: flixos90's profile flixos90 Owned by: stevenlinx's profile stevenlinx
Milestone: 4.9 Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: good-first-bug has-patch commit
Focuses: docs, multisite Cc:


There are several issues with the documentation for domain_exists() and its filter:

  • It's actually the domain-path combination that is checked. So the term "blogname" should probably be replaced with something like "domain-path combination".
  • In the filter documentation the term "site ID" should be used instead of "blog_id".
  • There's no description for the return type, and it's incorrectly highlighted as int where it should be int|null.
  • The $wpdb reference in the docblock can be removed as it's actually not used in the function.

Attachments (2)

41619.diff (1.6 KB) - added by stevenlinx 7 years ago.
41619.2.diff (1.8 KB) - added by stevenlinx 7 years ago.

Download all attachments as: .zip

Change History (11)

7 years ago

#1 @stevenlinx
7 years ago

  • Keywords has-patch added; needs-patch removed

#2 @flixos90
7 years ago

  • Owner set to stevenlinx
  • Status changed from new to assigned

Thanks for the patch @stevenlinx, looks good!

@DrewAPicture @jeremyfelt Any objections for the term "domain-path combination"? Is there a better one we should use?

#3 @DrewAPicture
7 years ago

Hmm. I think introducing the new terminology actually confuses it a bit more. You're correct that domain_exists() literally checks the domain path combination, though "domain-path combination" doesn't really convey what's being checked the way blogname sort-of does currently.

Rather than introduce new vernacular, I wonder if simply providing context for what "blogname" represents in the DocBlock description wouldn't be more consistent with the rest of core. For instance, in the newblogname hook doc, the explanation of what the [blog] name represents is pretty clear:

 * Filters the new site name during registration.
 * The name is the site's subdomain or the site's subdirectory
 * path depending on the network settings.
Last edited 7 years ago by DrewAPicture (previous) (diff)

#4 @flixos90
7 years ago

@DrewAPicture That makes sense. Let's go with "site name" then (replacing the old terminology "blogname") and provide an explanation like the one you shared.

7 years ago

#5 @flixos90
7 years ago

Changes look good @stevenlinx, thanks for continuing to work on it!

@DrewAPicture You're good with this?

This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.

7 years ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.

7 years ago

#8 @DrewAPicture
7 years ago

  • Keywords commit added

@flixos90 Looks good. All yours :-)

#9 @flixos90
7 years ago

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

In 41596:

Multisite: Fix documentation issues for domain_exists().

Props stevenlinx.
Fixes #41619.

Note: See TracTickets for help on using tickets.