Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Title says it all. We should remove this constant and replace with an '8.x' string. This is not hard to maintain and messes with unit tests.
Comment | File | Size | Author |
---|---|---|---|
#20 | 2067017-20.patch | 55.66 KB | damiankloip |
#20 | interdiff-2067017-20.txt | 41.43 KB | damiankloip |
#16 | 2067017-16.patch | 15.36 KB | damiankloip |
#16 | interdiff-2067017-16.txt | 6.96 KB | damiankloip |
#15 | 2067017-15.patch | 8.39 KB | damiankloip |
Comments
Comment #1
damiankloip CreditAttribution: damiankloip commentedComment #3
damiankloip CreditAttribution: damiankloip commentedComment #4
damiankloip CreditAttribution: damiankloip commentedForget it, Let's move it back to just views.
Comment #4.0
damiankloip CreditAttribution: damiankloip commentedUpdated issue summary.
Comment #5
dawehnerWhat about just removing that change ... then there is no installer problem.
Comment #6
damiankloip CreditAttribution: damiankloip commentedok, and just do this?
Comment #8
damiankloip CreditAttribution: damiankloip commentedOops.
Comment #10
damiankloip CreditAttribution: damiankloip commented#8: 2067017-8.patch queued for re-testing.
Comment #11
jibranRelated #2053519: Broken PHPUnit test with notices enabled for AJAX fails on View with "AJAX: Yes"
Comment #12
dawehnerHow many instances of the other constants do exist? Maybe we should ask someone whether they should be converted as well. Is there a tag for it?
Comment #13
webchickHm. Why only remove it from this one file? If we're moving it to the Drupal class, we ought to update all instances everywhere, no? Else we risk updating it in one place and not the other.
Comment #14
webchickComment #15
damiankloip CreditAttribution: damiankloip commentedok, fair enough. How about this?
Comment #16
damiankloip CreditAttribution: damiankloip commentedOh, and VERSION too as I've added that to Drupal too?
Comment #17
dawehnerI think this is good!
Comment #18
alexpottAre we sure that we want to maintain the constants in the Drupal class and bootstrap.inc - this seems are bit weird.
Is it not possible just to switch everything to use the class constants?
Comment #19
damiankloip CreditAttribution: damiankloip commentedWell, I think some constants are used before the class loader is initialized, so I don't think so...
If this is going to be an issue, I'm totally fine with just using the patch in #4, so at least I am happier for views tests. But it seems like other classes might as well get fixed along the way too. Plus, come on, that is not alot of code to maintain ;)
Comment #20
damiankloip CreditAttribution: damiankloip commentedOK, spoke to catch on IRC - We agreed that really there is no need for the current constants to exist, and their usage is pretty much limited to core anyway. So we can remove VERSION and DRUPAL_CORE_COMPATIBILITY and use the Drupal::* alterantives..
Comment #21
andypostis not @todo for this constant?
Comment #22
damiankloip CreditAttribution: damiankloip commentedThere are 2 other constant declarations below this one.
Comment #23
damiankloip CreditAttribution: damiankloip commentedComment #24
dawehnerSo, this is pretty solid.
Comment #25
catchCommitted/pushed to 8.x, thanks!
Comment #26
webchickComment #27
damiankloip CreditAttribution: damiankloip commentedOK, done: https://drupal.org/node/2082661
Comment #28
Gábor HojtsyRetitled change notice from "have been removed" to "have moved". They are not removed, you can find them at a different place.
Comment #29
damiankloip CreditAttribution: damiankloip commentedFair enough.
Comment #30
webchickLooks good, although, ugh. #2053489: Standardize on \Drupal throughout core
Comment #31
alexpottSo this has broken Drush - see https://github.com/drush-ops/drush/pull/65
Comment #32
greg.1.anderson CreditAttribution: greg.1.anderson commentedThe problem isn't so much that Drush has broken; Drush can change. The problem is that the fix is awkward for Drush.
See: https://github.com/drush-ops/drush/pull/65
Could we perhaps put the version in composer.json, as David Reid suggested? This is a standard attribute that's widely used; seems like it would be very valuable for Drupal to maintain its version here as well, for constancy. Side benefit: it would be good for Drush.
Comment #33
greg.1.anderson CreditAttribution: greg.1.anderson commentedI created a follow-on issue, #2082931: Store Drupal version in composer.json .
Comment #34.0
(not verified) CreditAttribution: commentedUpdated issue summary.