drupal_initialize_variables -> drupal_initialize_php

Really, this function has nothing to do with Drupal variables. It's all about PHP globals and ini things.

conf_init -> drupal_initialize_settings

This reads settings.php. Let's unify terminology by naming the function similarly.

variable_init -> variable_initialize

We're moving toward spelling out "initialize" elsewhere, so we should do so here, too.

CommentFileSizeAuthor
#9 517542-fix.patch649 bytesdamien tournoud
#4 rename-bootstrap-functions.patch14.78 KBdavid strauss
#2 rename-bootstrap-functions.patch14.83 KBdavid strauss
rename-bootstrap-functions.patch1.99 KBdavid strauss

Comments

Crell’s picture

Crell commented

+1 on concept. The second two I agree with entirely.

I'm not sure that drupal_initialize_php is the best name for the first function. It's setting defaults and configuration for both the $_SERVER super-global environment object and various php_ini() settings. So, maybe _php is the right suffix there? What about drupal_initialize_environment()? I'm not sure here either way, just bringing it up.

There's also drupal_session_initialize(). Should the name patterns match?

We should probably also fix init_theme() for the same reasons mentioned above. drupal_initialize_theme() / drupal_theme_initialize() ?

david strauss’s picture

david strauss
he/him
commented
Title: Rename bootstrap functions for clarity » Rename initialization functions for clarity
StatusFileSize
new rename-bootstrap-functions.patch14.83 KB

Here's a much-expanded patch.

dries’s picture

dries commented
Title: Rename initialization functions for clarity » Rename bootstrap functions for clarity
Status: Needs review » Reviewed & tested by the community

Much better, IMO. Can try to commit it later today.

david strauss’s picture

david strauss
he/him
commented
Title: Rename bootstrap functions for clarity » Rename initialization functions for clarity
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new rename-bootstrap-functions.patch14.78 KB

Here's an update without silly renaming of the block module theme function.

Crell’s picture

Crell commented
Status: Needs review » Reviewed & tested by the community

Yay, clarity!

damien tournoud’s picture

damien tournoud commented

Nice!

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

damien tournoud’s picture

damien tournoud commented
Category: task » bug
Priority: Normal » Critical
Status: Fixed » Active

This broke everything badly, because a variable_init() call was not converted in DrupalWebTestCase:

rename-bootstrap-functions.patch	passed	Passed: 153 passes, 0 fails, 0 exceptions Detailed results

^ Only 153 passes is a sign.

damien tournoud’s picture

damien tournoud commented
Status: Active » Needs review
StatusFileSize
new 517542-fix.patch649 bytes
david strauss’s picture

david strauss
he/him
commented

Does the test bot not catch fatal errors from calling undefined functions?

david strauss’s picture

david strauss
he/him
commented
Status: Needs review » Reviewed & tested by the community

DamZ's change should definitely go in. Not sure how I missed that one.

damien tournoud’s picture

damien tournoud commented

See #443154: Fatal errors in tests not reported as failures for the parent issue. I consider this a bug in PIFR, but there are some disagreement there.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed. Sorry!

damien tournoud’s picture

damien tournoud commented

yay! for a fixed core.

Status: Fixed » Closed (fixed)

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