WordPress.org

Make WordPress Core

Opened 6 weeks ago

Last modified 2 days ago

#41619 assigned defect (bug)

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
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 5 weeks ago.
41619.2.diff (1.8 KB) - added by stevenlinx 4 weeks ago.

Download all attachments as: .zip

Change History (9)

@stevenlinx
5 weeks ago

#1 @stevenlinx
5 weeks ago

  • Keywords has-patch added; needs-patch removed

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

#4 @flixos90
5 weeks 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
4 weeks ago

#5 @flixos90
4 weeks 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.


3 weeks ago

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


2 days ago

Note: See TracTickets for help on using tickets.