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.

Files: 
CommentFileSizeAuthor
#20 2067017-20.patch55.66 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 58,479 pass(es).
[ View ]
#20 interdiff-2067017-20.txt41.43 KBdamiankloip
#16 2067017-16.patch15.36 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 57,977 pass(es).
[ View ]
#16 interdiff-2067017-16.txt6.96 KBdamiankloip
#15 2067017-15.patch8.39 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 58,001 pass(es).
[ View ]
#15 interdiff-2067017-15.txt7.42 KBdamiankloip
#8 2067017-8.patch1000 bytesdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 58,423 pass(es).
[ View ]
#8 interdiff-2067017-8.txt349 bytesdamiankloip
#6 2067017-6.patch1007 bytesdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#4 vdc.remove-view-constant.patch562 bytesdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 58,059 pass(es).
[ View ]
#1 vdc.constants-bootstrap.patch2.15 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Comments

StatusFileSize
new2.15 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Status:Needs review» Needs work

The last submitted patch, vdc.constants-bootstrap.patch, failed testing.

Component:views.module» base system

Title:Convert DRUPAL_CORE_COMPATIBILITY and VERSION constants in bootstrap.inc to Drupal class constantsRemove usage of DRUPAL_CORE_COMPATIBILITY constant in Drupal\views\Plugin\Core\Entity\View
Component:base system» views.module
Status:Needs work» Needs review
StatusFileSize
new562 bytes
PASSED: [[SimpleTest]]: [MySQL] 58,059 pass(es).
[ View ]

Forget it, Let's move it back to just views.

Issue summary:View changes

Updated issue summary.

+++ b/core/includes/bootstrap.inc
@@ -31,12 +31,12 @@
-const VERSION = '8.0-dev';
+const VERSION = Drupal::VERSION;
...
-const DRUPAL_CORE_COMPATIBILITY = '8.x';
+const DRUPAL_CORE_COMPATIBILITY = Drupal::CORE_COMPATIBILITY;

What about just removing that change ... then there is no installer problem.

StatusFileSize
new1007 bytes
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

ok, and just do this?

Status:Needs review» Needs work

The last submitted patch, 2067017-6.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new349 bytes
new1000 bytes
PASSED: [[SimpleTest]]: [MySQL] 58,423 pass(es).
[ View ]

Oops.

Status:Needs review» Needs work
Issue tags:-VDC

The last submitted patch, 2067017-8.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+VDC

#8: 2067017-8.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

How 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?

Hm. 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.

Status:Reviewed & tested by the community» Needs review

Title:Remove usage of DRUPAL_CORE_COMPATIBILITY constant in Drupal\views\Plugin\Core\Entity\ViewRemove usage of DRUPAL_CORE_COMPATIBILITY constant in classes
StatusFileSize
new7.42 KB
new8.39 KB
PASSED: [[SimpleTest]]: [MySQL] 58,001 pass(es).
[ View ]

ok, fair enough. How about this?

Title:Remove usage of DRUPAL_CORE_COMPATIBILITY constant in classesRemove usage of DRUPAL_CORE_COMPATIBILITY/VERSION constant in classes
StatusFileSize
new6.96 KB
new15.36 KB
PASSED: [[SimpleTest]]: [MySQL] 57,977 pass(es).
[ View ]

Oh, and VERSION too as I've added that to Drupal too?

Status:Needs review» Reviewed & tested by the community

I think this is good!

Status:Reviewed & tested by the community» Needs review

Are 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?

Well, 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 ;)

StatusFileSize
new41.43 KB
new55.66 KB
PASSED: [[SimpleTest]]: [MySQL] 58,479 pass(es).
[ View ]

OK, 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..

+++ b/core/modules/views/tests/Drupal/views/Tests/Plugin/Block/ViewsBlockTest.php
@@ -12,9 +12,6 @@
// @todo Remove this once the constant got converted.
-if (!defined('DRUPAL_CORE_COMPATIBILITY')) {
-  define('DRUPAL_CORE_COMPATIBILITY', '8.x');
-}

is not @todo for this constant?

There are 2 other constant declarations below this one.

Title:Remove usage of DRUPAL_CORE_COMPATIBILITY/VERSION constant in classesRemove usage of DRUPAL_CORE_COMPATIBILITY/VERSION constants

Status:Needs review» Reviewed & tested by the community

So, this is pretty solid.

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

Title:Remove usage of DRUPAL_CORE_COMPATIBILITY/VERSION constantsChange notice: Remove usage of DRUPAL_CORE_COMPATIBILITY/VERSION constants
Priority:Normal» Critical
Status:Fixed» Active
Issue tags:+Needs change record

Status:Active» Needs review

Retitled change notice from "have been removed" to "have moved". They are not removed, you can find them at a different place.

Fair enough.

Title:Change notice: Remove usage of DRUPAL_CORE_COMPATIBILITY/VERSION constantsRemove usage of DRUPAL_CORE_COMPATIBILITY/VERSION constants
Priority:Critical» Normal
Status:Needs review» Fixed
Issue tags:-Needs change record

So this has broken Drush - see https://github.com/drush-ops/drush/pull/65

The 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.

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

Issue summary:View changes

Updated issue summary.