Move node constants to NodeInterface

Do you mean to move only NODE_PUBLISHED, or may be to move all constants from node.module to NodeInterface?
Also i think STATUS_PUBLISHED is a little more consistent.

Moved. Named as:

• NodeInterface::STATUS_NOT_PUBLISHED
• NodeInterface::STATUS_PUBLISHED
• NodeInterface::OPTION_NOT_PROMOTED
• NodeInterface::OPTION_PROMOTED
• NodeInterface::OPTION_NOT_STICKY
• NodeInterface::OPTION_STICKY
• NodeInterface::ACCESS_ALLOW
• NodeInterface::ACCESS_DENY
• NodeInterface::ACCESS_IGNORE

Yep we need a change record for this.

Draft change record created at https://www.drupal.org/node/2316145

Reroll.

Reroll.

Re-roll the patch.

Reroll attached.

DrupalConLA first time reviewer. Patch #26 appears to need to be re-rolled. Since this touches so many places I will not attempt to reroll.

While re-rolling a new patch I:
1. Downloaded the most recent patch (#26) which failed to apply.
2. Created a new patch after checking out latest changes from 8.0.x branch.

Recreating patch as previous one failed for having use Drupal\node\NodeInterface; statement twice in a file.

The patch was applied successfully.

Unfortunately this change does not pass the beta evaluation. See https://www.drupal.org/core/beta-changes for more information. It would possible to deprecate the constants and add the new constants to NodeInterface - however even this would not be permitted in beta because it would still be a normal task with no tangible benefits at this point in the Drupal 8 release cycle. Given that doing this change is possible in a non bc breaking manner I'm going to postpone this to 8.1

+++ b/core/modules/node/node.module
@@ -31,36 +31,6 @@
 /**
- * Denotes that the node is not published.
- */
-const NODE_NOT_PUBLISHED = 0;
-
-/**
- * Denotes that the node is published.
- */
-const NODE_PUBLISHED = 1;
-
-/**
- * Denotes that the node is not promoted to the front page.
- */
-const NODE_NOT_PROMOTED = 0;
-
-/**
- * Denotes that the node is promoted to the front page.
- */
-const NODE_PROMOTED = 1;
-
-/**
- * Denotes that the node is not sticky at the top of the page.
- */
-const NODE_NOT_STICKY = 0;
-
-/**
- * Denotes that the node is sticky at the top of the page.
- */
-const NODE_STICKY = 1;

Removing these constants will break existing Drupal 8 contrib.

Unpostponing. We can deprecate them now.

.

Updated the IS. I didn't add an interdiff as is not relevant.

.

@vaplas, thank you. Indeed those were unused statements.

UPDATE: interdiff-37-to-40.patch is in fact the interdiff

This patch revealed a bug in ModuleInstaller. The problem is that, on install, the module files "$module.module" and "$module.install" are loaded before the the module's PSR4 namespaces are registered. In this case, the code that is executed on file load (like constant declarations) cannot benefit from the new namespaces.

Ouch, forgot a full qualified namespace inside.

+++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
@@ -175,12 +175,13 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
         // Update the module handler in order to load the module's code.
         // This allows the module to participate in hooks and its existence to
-        // be discovered by other modules.
+        // be discovered by other modules. The module code ("$module.module" and
+        // "$module.install" files) are loaded only after the kernel rebuild in
+        // order to make the PSR4 namespaces provided by this module available
+        // to its own code.
         // The current ModuleHandler instance is obsolete with the kernel
         // rebuild below.
         $this->moduleHandler->setModuleList($module_filenames);
-        $this->moduleHandler->load($module);
-        module_load_install($module);
 
         // Clear the static cache of system_rebuild_module_data() to pick up the
         // new module, since it merges the installation status of modules into
@@ -190,6 +191,10 @@ public function install(array $module_list, $enable_dependencies = TRUE) {

@@ -190,6 +191,10 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
         // Update the kernel to include it.
         $this->updateKernel($module_filenames);
 
+        // Load the module code ("$module.module" and "$module.install" files).
+        $this->moduleHandler->load($module);
+        module_load_install($module);

This change is a concern because there's a small possibility of regression here. It is possible a container compiler pass might use code from the .module file. I can't see a way around this. Going to get other framework manager / release manager opinions. But this part definitely deserves its own change record.

We can easily work around this by simply keeping the actual values in the old constants, that's what we did before.

Then we can move that change into a separate issue.

Yeah completely agreed with #48, there's no reason to get the constants from the interface for deprecated code and we've never tried to do that before.

It's worth a follow-up to look at the wider issue, but let's look at that separately.

Hardcoded values for deprecated constants.

Change record added: https://www.drupal.org/node/2843370

@claudiu.cristea I updated the old CR (https://www.drupal.org/node/2316145) with you improvements and deleted your old one. We only needed another CR for the module installer changes that no longer are a part of this.

We only needed another CR for the module installer changes that no longer are a part of this.

@alexpott, but that needs also its own issue, right?

Split into #2843457: Provide PSR4 namespaces before loading the module code in ModuleInstaller.

+++ b/core/modules/node/src/Entity/Node.php
@@ -228,7 +228,7 @@ public function isPromoted() {
+    $this->set('promote', $promoted ? NodeInterface::OPTION_PROMOTED : NodeInterface::OPTION_NOT_PROMOTED);

Why do these need to be OPTION_PROMOTED and OPTION_NOT_PROMOTED? Just PROMOTED and NOT_PROMOTED would do?

Same with PUBLISHED/NOT_PUBLISHED too really.

Fixed #57. Back to RTBC as it was only a simple renaming. Fixed also the IS.

Committed/pushed to 8.4.x and cherry-picked to 8.3.x. Thanks!