Opened 2 years ago

Closed 2 years ago

#16682 closed enhancement (wontfix)

Actually set visibility keywords on methods and properties

Reported by: scribu Owned by:
Priority: normal Milestone:
Component: General Version:
Severity: normal Keywords: php5 has-patch
Cc:

Description

There are a lot classes in WP with the @access private comment, but without the actual keyword in the code, due to the previous PHP 4 compatiblity requirement.

This should be fixed.

Attachments (2)

16682.diff (93.5 KB) - added by aaroncampbell 2 years ago.
Everything
16682.2.diff (10.2 KB) - added by aaroncampbell 2 years ago.
Only class-wp-posts-list-table.php and class-wp-list-table.php

Download all attachments as: .zip

Change History (14)

  • Type changed from defect (bug) to enhancement
  • Keywords 3.2-early removed
  • Milestone changed from Future Release to 3.2

Westi says: small patches welcome

I think good initial candidates for this are the WP_List_Table classes and the WP_Widget class, since they're relatively new.

So, the guideline is: new code yes. old code no.

That leaves WP_List_Table classes and WP_Admin_Bar.

Will have to check the Fluency Admin plugin as well as others to see what we can reasonably mark as private in WP_Admin_Bar.

Everything

More for reference (or as a starting point) that patch adds private, protected, or public to every class variable and method that had one specified with @access php doc. I did skip libraries that we don't maintain (like simplepress, phpmailer, atomlib, etc).

I'll assume you made that patch before reading the comments. :P

Yes, I did most of it earlier today just because I was curious whether things would catastrophically break. I finished it up after the meeting, and figured why not at least upload it. I suppose I could apply the patch to another trunk checkout and then revert the files we don't want patched and create another patch from that.

Only class-wp-posts-list-table.php and class-wp-list-table.php

The new patch should be good. WP_Admin_Bar doesn't actually have any @access specifications, so I suppose we could set them all to public if we want.

  • Keywords has-patch added
  • Owner set to scribu
  • Status changed from new to reviewing
  • Owner scribu deleted
  • Milestone changed from 3.2 to Future Release

Didn't get tested before freeze. Next time.

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from reviewing to closed

WP_Admin_Bar doesn't have any @access specifications, the list table classes are private in their entirety to undergo a near-future re-architecture where this will surely be considered, and we decided not to touch existing classes for compatibility reasons. So closing as wontfix.

Note: See TracTickets for help on using tickets.