Skip to:
Content

Opened 4 months ago

Closed 2 months ago

Last modified 2 months ago

#7335 closed enhancement (wontfix)

Anonymize BP Core Dependency functions file.

Reported by: tw2113 Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Core Keywords: close
Cc:

Description

When JJJ first wrote up the bp-core-dependency.php file, he left a note at the top regarding utilizing PHP 5.3's anonymous function capabilities for the file.

This ticket is meant to help move that to a reality. We will need to keep in mind anyone who may be calling the functions directly, and help them out the best we can.

Attachments (3)

7335-anonymous-actions.diff (7.0 KB) - added by tw2113 4 months ago.
7335-anonymous-actions.2.diff (15.1 KB) - added by tw2113 4 months ago.
7335-deprecated-notices.diff (22.4 KB) - added by tw2113 4 months ago.

Download all attachments as: .zip

Change History (17)

#1 @tw2113
4 months ago

Add deprecated function notices to functions in the BP Core Dependency declarations.

#2 @tw2113
4 months ago

Done all the way to right before bp_ready() with the attached diff file.

This ticket was mentioned in Slack in #buddypress by tw2113. View the logs.


4 months ago

#4 @tw2113
4 months ago

The bp_allowed_themes() function is an odd one, in that it's present, and not hooked into anything, and doesn't appear to be used in core anywhere.

Updated diff pending, once I have a chance to review changes.

#5 @tw2113
4 months ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 2.8

Added a more complete anonymized actions/filters patch, as well as a patch that deprecates all the old functions.

Left bp_allowed_themes() alone like noted earlier.

#6 @DJPaul
3 months ago

  • Keywords 2nd-opinion added

Looks good but it makes these files much longer and adds functions in a file that was originally designed to not really have (m)any functions in. The priorities are also a little harder to read this way. Would be interested in opinions from other people.

Maybe assign anonymous functions to variables and use those to replace the add_action lines, keeping them short and tidy. Maybe this is just me over-reacting to a niggle.

#7 @tw2113
3 months ago

More than willing to get some second opinions on this one as well.

#8 @tw2113
3 months ago

For an example highlighting one of Paul's thoughts above, the variable storage would look like this:

<?php
$bp_ready = function() {
        /**
         * Fires on the 'wp' hook, which runs after BP is set up and the page is about to render.
         *
         * @since 1.6.0
         */
        do_action( 'bp_ready' );
};

...

add_action( 'wp', $bp_ready, 10 );

#9 @boonebgorges
3 months ago

What is the benefit of switching to anonymous functions, especially if we're keeping the old ones around?

#10 @r-a-y
3 months ago

Yeah, I do not see much of a point to moving to anonymous functions here. Unless there is a gigantic performance boost!

Also, it's harder to backtrace anonymous functions.

#11 @tw2113
3 months ago

It was pretty much just a response to the note from JJJ in the file. If you'd prefer not proceed with this one and wontfix, no hard feelings.

This ticket was mentioned in Slack in #buddypress by boone. View the logs.


2 months ago

#13 @tw2113
2 months ago

  • Keywords close added; has-patch 2nd-opinion removed
  • Milestone 2.8 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

After discussion in the dev chat on 2016-12-21, we've decided to mark as wontfix for the time being.

#14 @boonebgorges
2 months ago

In 11364:

Remove @todo related to migrating bp-core-dependency functions to closures.

See #7299. See #7335.

Note: See TracTickets for help on using tickets.