WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#41452 closed task (blessed) (fixed)

Remove @access tags from core DocBlocks

Reported by: DrewAPicture Owned by: DrewAPicture
Milestone: 4.9 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch
Focuses: docs Cc:
PR Number:

Description

Per discussion in #core-docs last week, we're finally to the point where we can remove @access notations from DocBlocks throughout core, with a few exceptions:

  • Any standalone core functions marked private
  • Any classes marked private (such as the List Table classes, for instance)

The parser that builds the Code Reference already references actual method visibility as derived with phpDocumentor, meaning that output will be unchanged following the removals.

In terms of anything marked private (including the aforementioned exceptions) the Code Reference theme already handles displaying a specially-designed callout box.

Attachments (4)

41452.diff (195.6 KB) - added by DrewAPicture 3 years ago.
41452.2.diff (93.4 KB) - added by DrewAPicture 3 years ago.
41452-wp-admin.diff (93.6 KB) - added by DrewAPicture 3 years ago.
wp-admin
41452-wp-includes.diff (398.2 KB) - added by DrewAPicture 3 years ago.
wp-includes

Download all attachments as: .zip

Change History (17)

@DrewAPicture
3 years ago

#1 @DrewAPicture
3 years ago

  • Owner set to DrewAPicture
  • Status changed from new to accepted

41452.diff covers classes in wp-includes.

@DrewAPicture
3 years ago

#2 @DrewAPicture
3 years ago

41452.2.diff covers classes in wp-admin

@DrewAPicture
3 years ago

wp-admin

@DrewAPicture
3 years ago

wp-includes

#3 @DrewAPicture
3 years ago

The first passes appeared to miss some files. Maybe I was accidentally skipping them when I was clicking though or something.

Anyway, 41452-wp-admin.diff covers wp-admin, and 41452-wp-includes.diff covers wp-includes.

#4 in reply to: ↑ description @SergeyBiryukov
3 years ago

Replying to DrewAPicture:

Per discussion in #core-docs last week, we're finally to the point where we can remove @access notations from DocBlocks throughout core, with a few exceptions:

  • Any standalone core functions marked private
  • Any classes marked private (such as the List Table classes, for instance)

Great idea, having @access public in the DocBlocks is redundant, as it's already in the code itself.

#5 @DrewAPicture
3 years ago

In 41161:

Docs: Remove @access notations from method DocBlocks in wp-admin/* classes.

Prior to about 2013, many class methods lacked even access modifiers which made the @access notations that much more useful. Now that we've gotten to a point where the codebase is more mature from a maintenance perspective and we can finally remove these notations. Notable exceptions to this change include standalone functions notated as private as well as some classes still considered to represent "private" APIs.

See #41452.

#6 @DrewAPicture
3 years ago

In 41162:

Docs: Remove @access notations from method DocBlocks in wp-includes/* classes.

Prior to about 2013, many class methods lacked even access modifiers which made the @access notations that much more useful. Now that we've gotten to a point where the codebase is more mature from a maintenance perspective and we can finally remove these notations. Notable exceptions to this change include standalone functions notated as private as well as some classes still considered to represent "private" APIs.

See #41452.

#7 @DrewAPicture
3 years ago

Note: the inline documentation standards for PHP have been updated to reflect this change.

#8 @DrewAPicture
3 years ago

@jrf: Leaving a note for you here assuming you may need to make adjustments to any WPCS rules around @access tags.

@coffee2code: I grokked the parser and wporg-developer code last week and I think we're good in terms of parsing since we store the method visibility in meta and already check against it with a fallback to the access tag. The fallback can continue to handle the exceptions outlined above.

#9 @jrf
3 years ago

@DrewAPicture At this moment the WPCS Docs ruleset does not (yet) check these kind of things in that much detail.
When the ruleset is improved in the future, this will need to be taken into account by whoever does the improving.

#10 @DrewAPicture
3 years ago

Made typo in [41168] which was also against this ticket.

#11 @DrewAPicture
2 years ago

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

This is all done. If we come across any that were missed, let's handle them in separate tickets.

#12 @SergeyBiryukov
2 years ago

In 42748:

Docs: Remove @access notation from WP_Taxonomy::$meta_box_sanitize_cb.

See #42505, #41452.

#13 @SergeyBiryukov
2 years ago

In 42749:

Twenty Fourteen: Remove redundant @access and @static notations from classes.

Props birgire.
See #41452, #42803, #42505.

Note: See TracTickets for help on using tickets.