Download & Extend

get bootstrap phase is uuuuuuuugly

Project:Drupal core
Version:7.x-dev
Component:base system
Category:bug report
Priority:critical
Assigned:chx
Status:closed (fixed)

Issue Summary

That's some ugly code. We can do better.

AttachmentSizeStatusTest resultOperations
get_bootstrap_phase_no_global_oh_my.patch1.88 KBIgnored: Check issue status.NoneNone

Comments

#1

DamZ complained that I return the phase we are in and not the one we completed. But if we are to keep the completed_phase then we can refactor the whole function to something more simpler...

AttachmentSizeStatusTest resultOperations
get_bootstrap_phase_collateral.patch1.85 KBIgnored: Check issue status.NoneNone

#2

Status:needs review» reviewed & tested by the community

$ egrep -r _drupal_current_bootstrap_phase .
./includes/bootstrap.inc:    global $_drupal_current_bootstrap_phase;
./includes/bootstrap.inc:    $_drupal_current_bootstrap_phase = $current_phase;
./includes/bootstrap.inc:  global $_drupal_current_bootstrap_phase;
./includes/bootstrap.inc:  return $_drupal_current_bootstrap_phase;

$_drupal_current_bootstrap was not used anywhere outside those two functions.

I have nothing to add...

#3

Status:reviewed & tested by the community» fixed

Committed to CVS HEAD. Thanks all.

#4

Status:fixed» active

This broke the installer.

#5

Priority:normal» critical
Status:active» needs review

Pfff.

Note to self: review chx' patches with a critical eye.

AttachmentSizeStatusTest resultOperations
323372-bootstrap-fix.patch950 bytesIgnored: Check issue status.NoneNone

#6

Better yet.

AttachmentSizeStatusTest resultOperations
323372-bootstrap-fix.patch1018 bytesIgnored: Check issue status.NoneNone

#7

Should we also remove the now-unnecessary drupal_get_bootstrap_phase function or separate patch?

#8

No we don't need to remove it. It's a clean API, for once, please don't remove it :)

#9

Status:needs review» reviewed & tested by the community

Last two patches fix issue and get all passes.

http://testing.drupal.org/pifr/node/1/323372

or even like:
http://testing.drupal.org/pifr/file/drupal.org/323372-bootstrap-fix.patch

When this gets in it would be nice if I was notified so I can update t.d.o. We seem to be getting a number of these that I have to hack t.d.o in the mean time.

#10

Status:reviewed & tested by the community» fixed

Committed to CVS HEAD. Thanks! :)

#11

Status:fixed» closed (fixed)

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