get bootstrap phase is uuuuuuuugly
chx - October 19, 2008 - 17:46
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | critical |
| Assigned: | chx |
| Status: | closed |
Description
That's some ugly code. We can do better.
| Attachment | Size |
|---|---|
| get_bootstrap_phase_no_global_oh_my.patch | 1.88 KB |
#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_phasethen we can refactor the whole function to something more simpler...#2
$ 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_bootstrapwas not used anywhere outside those two functions.I have nothing to add...
#3
Committed to CVS HEAD. Thanks all.
#4
This broke the installer.
#5
Pfff.
Note to self: review chx' patches with a critical eye.
#6
Better yet.
#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
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
Committed to CVS HEAD. Thanks! :)
#11
Automatically closed -- issue fixed for two weeks with no activity.