Problem/Motivation

There's lots of commented-out code in the core codebase, especially. Code might be commented for a lot of reasons -- missed cleanup on a patch, accidentally leaving something commented out after debugging, skipped tests for later functionality -- but it is almost always wrong and a code smell that can indicate bugs, incomplete APIs, etc.

Any commented-out code purposefully left for later development should be accompanied by an @todo linking to the issue that will address the point in the future.

Proposed resolution

  1. Find commented-out code in the Drupal core codebase.
  2. In each case, use git log -L for the commented lines to figure out when and why the code was commented in the first place, and how the lines around it have changed.
  3. Do research into the issues that you find. Read them carefully and check comments and interdiffs for how and why the code got commented out.
  4. Based on this, make a recommendation as to what to do. Possibilities:
    • The commented code should be accompanied with an @todo and followup issue (find or file it).
    • The commented code is leftover cruft that can be removed. (If this is the case, read the code very carefully to see what the purpose of the code was and that it's no longer needed! Check for test coverage of the code.
    • For test code, commented test methods should possibly be marked skipped instead.
  5. Check with the maintainers for the subsystem to make sure your changes are correct.

Remaining tasks

Open issues

Where to find other issues

Some examples from #2715485: Fix 'Drupal.Commenting.InlineComment.NoSpaceBefore' coding standard and the related issues:


+++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php
@@ -111,7 +111,7 @@ public static function open(array &$connection_options = []) {
   //elseif (phpversion('pdo_pgsql') < 'version_this_was_fixed_in') {

+++ b/core/lib/Drupal/Core/Database/StatementInterface.php
@@ -37,7 +37,7 @@
 //protected function __construct(Connection $dbh);

+++ b/core/modules/field/tests/src/Functional/EntityReference/EntityReferenceAutoCreateTest.php
@@ -213,17 +213,17 @@ public function testMultipleTargetBundles() {
   //unset($handler_settings['auto_create_bundle']);
   //$field_config->setSetting('handler_settings', $handler_settings);
   //$field_config->save();
     //
   //$this->drupalGet('node/add/' . $this->referencingType);
   //$error_message = sprintf(
     //  "Create referenced entities if they don't already exist option is enabled but a specific destination bundle is not set. You should re-visit and fix the settings of the '%s' (%s) field.",
     //  $field_config->getLabel(),
     //  $field_config->getName()
   //);
   //$this->assertErrorLogged($error_message);

+++ b/core/modules/rdf/tests/src/Functional/EntityReferenceFieldAttributesTest.php
@@ -125,23 +125,23 @@ public function testNodeTeaser() {
+    // $this->assertTrue($graph->hasProperty($taxonomy_term_1_uri, 'http://www.w3.org/1999/02/22-rdf-syntax-ns#type', $expected_value), 'Taxonomy term type found in RDF output (skos:Concept).');
...
   //$this->assertTrue($graph->hasProperty($taxonomy_term_1_uri, 'http://www.w3.org/2000/01/rdf-schema#label', $expected_value), 'Taxonomy term name found in RDF output (rdfs:label).');
...
   //$this->assertTrue($graph->hasProperty($taxonomy_term_2_uri, 'http://www.w3.org/1999/02/22-rdf-syntax-ns#type', $expected_value), 'Taxonomy term type found in RDF output (skos:Concept).');
...
   //$this->assertTrue($graph->hasProperty($taxonomy_term_2_uri, 'http://www.w3.org/2000/01/rdf-schema#label', $expected_value), 'Taxonomy term name found in RDF output (rdfs:label).');

+++ b/core/modules/rdf/tests/src/Functional/StandardProfileTest.php
@@ -407,7 +407,7 @@ protected function assertRdfaArticleProperties($graph, $message_prefix) {
   //$this->assertEqual($graph->type($this->termUri), 'schema:Thing', 'Tag type was found (schema:Thing).');

+++ b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php
@@ -279,7 +279,7 @@ public function testPostDxWithoutCriticalBaseFields() {
   //$this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nentity_type: This value should not be null.\n", $response);

@@ -288,14 +288,14 @@ public function testPostDxWithoutCriticalBaseFields() {
     //$this->assertSame(500, $response->getStatusCode());
...
     //$this->assertSame("Error: Call to a member function get() on null\nDrupal\\comment\\Plugin\\Validation\\Constraint\\CommentNameConstraintValidator->getAnonymousContactDetailsSetting()() (Line: 96)\n", $e->getMessage());
     }
   //$response = $this->request('POST', $url, $request_options);
   //$this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nentity_type: This value should not be null.\n", $response);

@@ -303,7 +303,7 @@ public function testPostDxWithoutCriticalBaseFields() {
   //$this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nfield_name: This value should not be null.\n", $response);

+++ b/core/modules/update/update.module
@@ -349,7 +349,7 @@ function update_get_available($refresh = FALSE) {
     //update_create_fetch_task($project);

+++ b/core/modules/user/src/Tests/Views/AccessRoleTest.php
@@ -128,8 +128,8 @@ public function testRenderCaching() {
   //$this->assertTrue(in_array('user.roles', $build['#cache']['contexts']));
   //$this->assertEqual([], $build['#cache']['tags']);

+++ b/core/modules/views/tests/src/Functional/Plugin/ArgumentDefaultTest.php
@@ -131,11 +131,7 @@ public function testArgumentDefaultFixed() {
 //function testArgumentDefaultPhp() {}
-
 /**
  * Test node default argument.
  */

+++ b/core/tests/Drupal/KernelTests/Core/Database/LoggingTest.php
@@ -64,7 +64,7 @@ public function testEnableTargetLogging() {
   db_query('SELECT age FROM {test} WHERE name = :name', [':name' => 'Ringo'], ['target' => 'replica']);//->fetchCol();

+++ b/core/tests/Drupal/KernelTests/Core/Entity/Element/EntityAutocompleteElementFormTest.php
@@ -353,7 +353,7 @@ public function testEntityAutocompleteAccess() {
   //$form = $form_builder->getForm($this);

+++ b/core/tests/Drupal/Tests/Core/Entity/EntityTypeBundleInfoTest.php
@@ -100,7 +100,7 @@ protected function setUp() {
   //$container->get('typed_data_manager')->willReturn($this->typedDataManager->reveal());

Completed issues

None yet.

Comments

xjm created an issue. See original summary.

xjm’s picture

Issue summary: View changes

Making the summary a little more legible by removing diff noise from the other issue.

xjm’s picture

Issue summary: View changes

Removing the Diff examples because we should leave those alone, and added the taxonomy issue I filed.

mfernea’s picture

Issue summary: View changes

I added 3 new child issues and also mentioned them in the "Open issues" section.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

alexpott’s picture

Open #3177919: Uncomment code in EntityReferenceAutoCreateTest to address one of the remaining things.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.