Move some more functions from update.php to includes/update.inc

means that you require less to implement upgrade scripts.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gordon’s picture

Removed a little bit of debugging that I had added.

dww’s picture

Status: Needs review » Needs work

Sure, it'd be great to move those two functions (update_batch() and update_finished()) into includes/update.inc. A few problems/questions:

A) This hunk looks like stray debugging code that should be removed:

@@ -327,7 +277,7 @@ function update_check_requirements() {
 
 // Some unavoidable errors happen because the database is not yet up-to-date.
 // Our custom error handler is not yet installed, so we just suppress them.
-ini_set('display_errors', FALSE);
+//ini_set('display_errors', FALSE);
 
 // We prepare a minimal bootstrap for the update requirements check to avoid
 // reaching the PHP memory limit.

B) Why are we looking at $_POST directly here?

@@ -399,7 +349,7 @@ if ($update_access_allowed) {
 
     case 'Apply pending updates':
       if (isset($_GET['token']) && $_GET['token'] == drupal_get_token('update')) {
-        update_batch();
+        update_batch($_POST['start'], $base_url . '/update.php?op=results', $base_url . '/update.php');
         break;
       }

C) The PHP doc for the new parameters for update_batch() doesn't follow core's code style. The @param things should be indented 3 spaces from the *, and should be capitalized and written as a complete sentence with a period on the end. They should also wrap to 80 chars. "Start the update batch process" should also end in a period.

dww’s picture

Oh, whoops -- cross post. #1 already fixed (A). Thanks. ;) (B) and (C) are still open questions.

dww’s picture

In IRC, Gordon pointed out that the answer to (B) is because "exactly how it was being handled in update.php before I made the change". Confirmed. ;) So, it's clearly outside the scope of this issue to change raw $_POST, if it even needs to be changed at all. ;)

Therefore, this only needs work for the cosmetic reasons in (C)...

gordon’s picture

Status: Needs work » Needs review
FileSize
5.81 KB

I have cleaned up the above issues. The PHPDoc should be correct now.

dww’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
5.94 KB

Re-rolled to fix a few more PHP doc problems:
- Everything now wraps to 80 characters.
- Used complete sentences everywhere.
- Simplified some of the text.
- Fleshed out the description for update_finished() (a previously undocumented function).

Otherwise, I didn't touch any of the actual code. Tested and update.php still works great. Even though it's my own patch, I'm going to call it RTBC.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.