[wp-trac] Re: [WordPress Trac] #7875: consolidate plugin/theme/core upgrade/install functions

[WordPress Trac]

#7875: consolidate plugin/theme/core upgrade/install functions
-----------------------------+----------------------------------------------
 Reporter:  DD32             |       Owner:  anonymous              
     Type:  enhancement      |      Status:  new                    
 Priority:  highest omg bbq  |   Milestone:  2.8                    
Component:  Administration   |     Version:  2.7                    
 Severity:  major            |    Keywords:  has-patch needs-testing
-----------------------------+----------------------------------------------

Comment(by Denis-de-Bernardy):

 Haven't tested yet, but, a few questions/remarks while scanning the
 current patch. I suppose a few are due to my not being 100% up to date on
 this area of WP:

  - Changes to wp-admin/includes/class-wp-filesystem-base.php, as far as I
 can tell, move bits of code around *and* adds trailingslashit() calls.
 Might this not be rejected for backwards compatibility reasons due to the
 fact that it translates to API changes?

  - Maybe create a separate ticket, wp-admin/includes/class-wp-filesystem-
 direct.php cleanup + bug fix, for things you did to that file? Clearly
 it's a commit candidate in its own right, and a valid one at that. Else
 it'll get lost in this ticket and might never get fixed.

  - The same goes for wp-admin/includes/class-wp-filesystem-ftpext.php, wp-
 admin/includes/class-wp-filesystem-ftpsockets.php and wp-
 includes/update.php

  - WP_Upgrader::download_package(): "$download_file =
 download_url($package); //TODO, move to class?" -- probably not, as it
 would break backward compat

  - WP_Upgrader::unpack_package(): Maybe toss $package in the feedback?

  - WP_Upgrader::unpack_package(): You're not using the $delete_package
 variable

  - WP_Upgrader::install_package(): "Please always pass these" -- maybe
 switch the comment to: A WP error will be returned if these aren't set

  - WP_Upgrader::install_package(): Sometimes you check for if (
 $clear_working ) before deleting on error, and sometimes not. Shouldn't it
 always delete on error?

  - Theme_Upgrader::upgrade_strings(): "Maintainence mode?" -- yeyeye, +1
 to that.

  - Theme_Upgrader::install(): Probably just a copy/past, but... shouldn't
 the strings be $theme_files? :-)

  - @set_time_limit( 300 ); should be in the WP_Upgrade class. WP is
 like... ~2M nowadays? I've seen plugins and themes that were nearly as
 large or larger than that. My own theme currently is 5M when zipped. While
 we're at it, I think we could bump this to 600s. The current upgrade
 updates about 15M worth of files in under five minutes on most servers,
 but I did need to add a filter in a plugin so as to make things work on
 some slower servers in order to bump the limit to twice its value.

 In your opinion, how shareable are the check_for_update functions?

 D.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/7875#comment:14>
WordPress Trac <http://trac.wordpress.org/>
WordPress blogging software