Currently the VERSION constant is defined in system.module. This has an unfortunate effect that code (for example in a alternate cache include file) cannot determine the version of Drupal this is running. The version constant is not defined until the final phase of bootstrap, DRUPAL_BOOTSTRAP_FULL.

We should move this define to bootstrap.inc instead, so that the version of Drupal is always available from the start of the bootstrap process.

CommentFileSizeAuthor
#13 bootstrap_version-618938-d6-13.patch1.94 KBAlbert Volkman
#12 bootstrap_version-618938-11-d6.patch1.94 KBAlbert Volkman
#1 bootstrap-version-618938-1.patch1.73 KBpwolanin
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Active » Needs review
FileSize
bootstrap-version-618938-1.patch1.73 KB

Seems like we should move the minimum version info, etc as well.

simple patch - just cut from system.module and paste into bootstrap.inc

horncologne’s picture

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

Looking good! Great idea, btw.

chx’s picture

chx CreditAttribution: chx commented

That's OK. The performance hit resulting from this code movement is likely to be negligible small. (However, APC has a speedup for defines and there is a hidef extension, too. But I think we still do not have enough defines for a hit.)

bjaspan’s picture

bjaspan CreditAttribution: bjaspan commented

I certainly approve of the idea. Is it bikeshedding to suggest that the minimum support version of individual database engines does not belong at the same level in bootstrap?

pwolanin’s picture

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

Well, it seemed reasonable to me that we'd want to have that information available in DRUPAL_BOOTSTRAP_DATABASE. I guess you could make the case for moving it to database.inc instead, but keeping it together seems more sensible to me in terms of finding it in the codebase.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Needs review » Reviewed & tested by the community
Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Version: 7.x-dev » 6.x-dev
Status: Fixed » Patch (to be ported)
pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Version: 6.x-dev » 7.x-dev
Status: Patch (to be ported) » Reviewed & tested by the community

oops - looks like there were extra changes in the commit: http://drupal.org/cvs?commit=281932

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Yep, that was by accident. I already updated the other issue.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Version: 7.x-dev » 6.x-dev
Status: Fixed » Patch (to be ported)

seems like it would make sense to do the same for 6 and 5?

Albert Volkman’s picture

Albert Volkman CreditAttribution: Albert Volkman commented
Status: Patch (to be ported) » Needs review
FileSize
bootstrap_version-618938-11-d6.patch1.94 KB

Not sure if there's still interest in this after 2+ years, but here's a D6 backport.

Albert Volkman’s picture

Albert Volkman CreditAttribution: Albert Volkman commented
FileSize
bootstrap_version-618938-d6-13.patch1.94 KB

Renaming patch.

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.