Skip to:

Opened 8 years ago

Closed 8 years ago

#7299 closed enhancement (fixed)

PHP 5.2 -> PHP 5.3 code improvements

Reported by: djpaul's profile DJPaul Owned by:
Milestone: 2.8 Priority: normal
Severity: normal Version:
Component: Core Keywords:


This ticket is to discuss asynchronously how we should start to use PHP 5.3 in the existing codebase. I do not think it is wise to just go and change everything possible in one go just because we can, but there are a few areas that I'd personally like to see update. It is a tricky balance and we should have some consensus amongst frequent contributors of how we should approach it.

Areas we could change:

  • Remove compatibility functions for PHP functions that don't exist in PHP 5.2. Reason would be to remove dead code branches which, as we'd never expect them to be used, would be missed in the future if some bug was found in them. I've only found one so far:
    • BP_Activity_Activity::array_replace_recursive.
  • Anonymous functions/closures.
    • See class BP_Core_Sort_By_Key_Callback
  • Using func_get_args as a function parameter.
    • Search for $func_args = func_get_args(); in BP.
  • Autoloader support.
    • All the do_nav_backcompat checks that Boone added a few versions back.
  • The functions in bp-core-dependency.php long have a note that says replace with anonymous functions.
    • This would be by design because unhooking these core BuddyPress actions would just break BuddyPress.

Change History (9)

#1 @DJPaul
8 years ago

The ones I am most keen on personally are array_replace_recursive, BP_Core_Sort_By_Key_Callback, and the autoloader changes. The others seem to be more-or-less code style improvements. IMO>

#2 @tw2113
8 years ago

Will likely need to include unit tests, but anywhere that uses create_function() would be a good place to look at, especially as you come across working in those areas.

#3 @boonebgorges
8 years ago

I'm going to remove the do_autoload checks throughout BP. This also involves removing all bp-*-classes.php files, since they're only loaded when ! do_autoload. This could cause problems for anyone who is calling require ... bp-*-classes.php, but those instances are probably already broken, since loading the classes.php file after autoload would cause duplicate class definitions. If we run into error reports, I suppose we can reintroduce these files (they'd be empty).

#4 @boonebgorges
8 years ago

In 11360:

Remove do_autoload checks and manual class loaders.

Our PHP 5.3 requirement means that spl_autoload_register() will
always be available.

See #7299.

#5 @boonebgorges
8 years ago

In 11361:

Remove conditional checks for initializing navigation back compat.

Nav back compat requires ArrayAccess, which is only guaranteed to
be available in PHP 5.3+.

See #7299.

#6 @boonebgorges
8 years ago

In 11362:

Use a closure for bp_sort_by_key() callback.

Previously, bp_sort_by_key() was refactored to avoid the use
of create_function(). See #6864. This refactoring needed to be
PHP 5.2-compatible, which required a workaround for the
unavailabilty of closures. This workaround can now be removed.

See #7299.

#7 @boonebgorges
8 years ago

In 11363:

Stop assigning func_get_args() to a variable.

This was a workaround for a syntax limitation in PHP 5.2.

See #7299.

#8 @boonebgorges
8 years ago

In 11364:

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

See #7299. See #7335.

#9 @boonebgorges
8 years ago

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

Everything on this list is now done (except for bp-core-dependency, which we decided we aren't going to do - see #7335). Let's close this for 2.8, and open separate tickets for more tasks against future release.

Note: See TracTickets for help on using tickets.