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
- Find commented-out code in the Drupal core codebase.
- 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.
- 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.
- 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.
- Check with the maintainers for the subsystem to make sure your changes are correct.
Remaining tasks
Search core for commented out code.
This grep, grep -r --include \*.php "^\s*//.*=.*;" core
finds commented out code in the following methods.
- \Drupal\Tests\ban\Functional\IpAddressBlockingTest::testIPAddressValidation
- \Drupal\field\Plugin\migrate\process\d6\FieldInstanceDefaults::transform
- \Drupal\field\Plugin\migrate\process\d6\FieldInstanceSettings::transform
- \Drupal\views\Plugin\views\query\Sql::execute
- \Drupal\Tests\views\Functional\Handler\FieldWebTest::testAlterUrl
- \Drupal\Tests\views_ui\Functional\DefaultViewsTest::testDefaultViews
- \Drupal\FunctionalTests\Core\Recipe\StandardRecipeInstallTest::setUpSite
- \Drupal\KernelTests\Core\Entity\Element\EntityAutocompleteElementFormTest::testEntityAutocompleteIdInput
- \Drupal\KernelTests\Core\Image\ToolkitGdTest::testGifTransparentImages
- \Drupal\Tests\Component\Annotation\Doctrine\DocParserTest::testSyntaxErrorWithUnknownCharacters
- \Drupal\Tests\Core\Database\ConditionTest::dataProviderTestCompileWithKnownOperators
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/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/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