WordPress.org

Make WordPress Core

Opened 10 months ago

Closed 9 months ago

#41619 closed defect (bug) (fixed)

Fix documentation issues for `domain_exists()`

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

Description

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 10 months ago.
41619.2.diff (1.8 KB) - added by stevenlinx 10 months ago.

Download all attachments as: .zip

Change History (11)

@stevenlinx
10 months ago

#1 @stevenlinx
10 months ago

  • Keywords has-patch added; needs-patch removed

#2 @flixos90
10 months 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
10 months 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 10 months ago by DrewAPicture (previous) (diff)

#4 @flixos90
10 months 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.

@stevenlinx
10 months ago

#5 @flixos90
10 months 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.


10 months ago

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


9 months ago

#8 @DrewAPicture
9 months ago

  • Keywords commit added

@flixos90 Looks good. All yours :-)

#9 @flixos90
9 months 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.