WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#34726 closed defect (bug) (fixed)

Clarify class variable documentation on WP_Query

Reported by: danielbachhuber Owned by: DrewAPicture
Milestone: 4.9 Priority: normal
Severity: normal Version:
Component: Query Keywords: good-first-bug has-patch
Focuses: docs Cc:

Description

Most of the PHPDoc begins with "Set if query" (e.g. "Set if query contains posts."). However, each of these variables is set. More correct documentation would be "True if query" or similar.

Attachments (5)

set_if_query_patch.diff (4.9 KB) - added by megane9988 4 years ago.
Set if query → True if query.
34726_2.diff (5.1 KB) - added by megane9988 4 years ago.
34726_3.diff (7.9 KB) - added by megane9988 4 years ago.
update patch
34726_4.diff (8.1 KB) - added by megane9988 4 years ago.
fix miss comment
34726.diff (7.8 KB) - added by DrewAPicture 3 years ago.

Download all attachments as: .zip

Change History (14)

#1 @DrewAPicture
4 years ago

  • Milestone changed from Future Release to Awaiting Review

More correct documentation would be to actually write summaries for the properties describing what they represent. "True if query" would be useful in the @var description, however.

@megane9988
4 years ago

Set if query → True if query.

#2 @megane9988
4 years ago

Can I fix this ticket?

#3 follow-up: @DrewAPicture
4 years ago

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

@megane9988 Thanks for the patch.

Unfortunately what needs to happen here is actually two-fold. The first, as I mentioned in comment:1 is that we need to write actual summaries for these properties. The second is that whether or not the properties default to true should not be in the summary, it should be add as a description for the @var tags in the property DocBlocks.

For instance, a valid DocBlock for the $is_single property might look like this:

<?php
/**
 * Used to determine whether the current query is for a single post.
 *
 * @since 1.5.0
 * @access public
 * @var bool True if the query is for a single post. Default false.
 */
public $is_single = false;

Assigning the ticket to mark the good-first-bug as claimed.

@megane9988
4 years ago

#4 in reply to: ↑ 3 @megane9988
4 years ago

@DrewAPicture Thanks for your advice.

I updated patch. I added Doc comment in first and second part.

If it is OK, I continue ather part.

#5 @DrewAPicture
4 years ago

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

@megane9988 Looks like a great start, yep!

@megane9988
4 years ago

update patch

@megane9988
4 years ago

fix miss comment

#6 @megane9988
4 years ago

@DrewAPicture I reuploaded patch. Check please.

#7 @DrewAPicture
4 years ago

  • Owner changed from megane9988 to DrewAPicture
  • Status changed from assigned to reviewing

@DrewAPicture
3 years ago

#8 @DrewAPicture
3 years ago

  • Milestone changed from Future Release to 4.9

#9 @DrewAPicture
3 years ago

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

In 40966:

Docs: Add more useful summaries to the DocBlocks for boolean $is_* properties in WP_Query.

Props megane9988 for the initial patch.
Fixes #34726.

Note: See TracTickets for help on using tickets.