Follow-up to #2572659: Fix 'Drupal.Commenting.InlineComment.SpacingAfter' coding standard

Part of #2571965: [meta] Fix PHP coding standards in core.

Approach

We are testing coding standards with PHP CodeSniffer, using the Drupal coding standards from the Coder module. Both of these packages are not installed in Drupal core. We need to do a couple of steps in order to download and configure them so we can run a coding standards check.

Step 1: Add the coding standard to the whitelist

Every coding standard is identified by a "sniff". For example, an imaginary coding standard that would require all llamas to be placed inside a square bracket fence would be called the "Drupal.AnimalControlStructure.BracketedFence sniff". There are dozens of such coding standards, and to make the work easier we have started by only whitelisting the sniffs that pass. For the moment all coding standards that are not yet fixed are simply skipped during the test.

Open the file core/phpcs.xml.dist and add a line for the sniff of this ticket. The sniff name is in the issue title. Make sure your patch will include the addition of this line.

Step 2: Install PHP CodeSniffer and the ruleset from the Coder module

Both of these packages are not installed by default in Drupal core, so we need to download them. This can be done with Composer, from the root folder of your Drupal installation:

$ composer require drupal/coder squizlabs/php_codesniffer
$ ./vendor/bin/phpcs --config-set installed_paths ../../drupal/coder/coder_sniffer

Once you have installed the phpcs package, you can list all the sniffs available to you like this:

$ ./vendor/bin/phpcs --standard=Drupal -e

This will give you a big list of sniffs, and the Drupal-based ones should be present.

Step 3: Prepare the phpcs.xml file

To speed up the testing you should make a copy of the file phpcs.xml.dist (in the core/ folder) and save it as phpcs.xml. This is the configuration file for PHP CodeSniffer.

We only want this phpcs.xml file to specify the sniff we're interested in. So we need to remove all the rule items, and add only our own sniff's rule. Rule items look like this:

<rule ref="Drupal.Commenting.InlineComment.SpacingBefore"/>

Remove all of them, and add only the sniff from this issue title. This will make sure that our tests run quickly, and are not going to contain any output from unrelated sniffs.

Step 4: Run the test

Now you are ready to run the test! From within the core/ folder, run the following command to launch the test:

$ cd core/
$ ../vendor/bin/phpcs -p

This takes a couple of minutes. The -p flag shows the progress, so you have a bunch of nice dots to look at while it is running.

Step 5: Fix the failures

When the test is complete it will present you a list of all the files that contain violations of your sniff, and the line numbers where the violations occur. You could fix all of these manually, but thankfully phpcbf can fix many of them. You can call phpcbf like this:

$ ../vendor/bin/phpcbf

This will fix the errors in place. You can then make a diff of the changes using git. You can also re-run the test with phpcs and determine if that fixed all of them.

CommentFileSizeAuthor
#76 2719649-76.patch64.73 KBquietone
#76 diff-75-76.txt2.7 KBquietone
#75 2719649-75.patch66.23 KBlongwave
#72 2719649-interdiff-68-72.txt48.82 KBcburschka
#72 2719649-72.patch65.62 KBcburschka
#68 2719649-68.patch36.93 KBnikitagupta
#66 2719649-66.patch17.26 KBnikitagupta
#65 2719649-65.patch18.28 KBnikitagupta
#55 2719649-55.patch55.31 KBharsha012
#49 2719649-49.patch163.54 KBjofitz
#49 interdiff-47-49.txt75.64 KBjofitz
#47 2719649-47.patch97.11 KBjofitz
#44 2719649-44.patch96.41 KBharsha012
#43 2719649-43.patch95.78 KBharsha012
#41 interdiff-2719649-39-41.txt8.51 KBharsha012
#41 2719649-41.patch124.25 KBharsha012
#39 2719649-39.patch121.7 KBharsha012
#37 interdiff-35-37.txt65.09 KBcburschka
#37 2719649-37.patch14.84 KBcburschka
#35 interdiff-32-35.txt21.89 KBharsha012
#35 2719649-35.patch52.23 KBharsha012
#32 2719649-32.patch27.76 KBharsha012
#30 2719649-30.patch150.06 KBharsha012
#26 2719649-26.patch149.82 KBjofitz
#23 fix-2719649-22.patch151 KBhitesh-jain
#22 fix-2719649-22.patch151 KBhitesh-jain
#16 2719649-16.patch151 KBrasikap
#13 2719649-13.patch151.06 KBrasikap
#10 Drupal.Commenting.InlineComment.SpacingBefore-2719649-10.patch150.85 KBvprocessor
#6 Drupal.Commenting.InlineComment.SpacingBefore-2719649-6.patch152.07 KBvprocessor
#4 Drupal.Commenting.InlineComment.SpacingBefore-2719649-4.patch530.79 KBvprocessor

Issue fork drupal-2719649

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost created an issue. See original summary.

andypost’s picture

vprocessor’s picture

Assigned: Unassigned » vprocessor
vprocessor’s picture

Assigned: vprocessor » Unassigned
Status: Active » Needs review
FileSize
530.79 KB

done

vprocessor’s picture

Assigned: Unassigned » vprocessor
Status: Needs review » Needs work
vprocessor’s picture

Assigned: vprocessor » Unassigned
Status: Needs work » Needs review
FileSize
152.07 KB

updated with parent patch

andypost’s picture

  1. +++ b/core/modules/field/src/Tests/FormTest.php
    @@ -230,14 +229,13 @@ function testFieldFormSingleRequired() {
    -//  function testFieldFormMultiple() {
    +// function testFieldFormMultiple() {
     //    $this->field = $this->field_multiple;
     //    $field_name = $this->field['field_name'];
     //    $this->instance['field_name'] = $field_name;
     //    FieldStorageConfig::create($this->field)->save();
     //    FieldConfig::create($this->instance)->save();
     //  }
    -
    

    I think that should be removed as whole

  2. +++ b/core/modules/menu_link_content/src/Tests/LinksTest.php
    @@ -53,8 +53,8 @@ function createLinkHierarchy($module = 'menu_test') {
         // - parent
         //   - child-1
    -    //      - child-1-1
    -    //      - child-1-2
    +    //     - child-1-1
    +    //       - child-1-2
         //   - child-2
    

    this indent is wrong

vprocessor’s picture

Assigned: Unassigned » vprocessor
vprocessor’s picture

Status: Needs review » Needs work
vprocessor’s picture

Assigned: vprocessor » Unassigned
Status: Needs work » Needs review
FileSize
150.85 KB

rerolled & fixed

Mile23’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
$ git apply Drupal.Commenting.InlineComment.SpacingBefore-2719649-10.patch 
error: core/modules/filter/tests/src/Kernel/FilterUnitTest.php: No such file or directory
error: patch failed: core/modules/simpletest/src/Tests/SimpleTestBrowserTest.php:124
error: core/modules/simpletest/src/Tests/SimpleTestBrowserTest.php: patch does not apply
error: core/modules/system/src/Tests/Common/TableSortExtenderUnitTest.php: No such file or directory
error: core/modules/system/src/Tests/File/DirectoryTest.php: No such file or directory
error: core/modules/system/src/Tests/File/UrlRewritingTest.php: No such file or directory
error: core/modules/system/src/Tests/Menu/MenuLinkTreeTest.php: No such file or directory
error: core/modules/system/src/Tests/Menu/MenuTreeStorageTest.php: No such file or directory
error: core/modules/system/src/Tests/Plugin/Condition/RequestPathTest.php: No such file or directory
error: core/modules/system/src/Tests/Routing/ContentNegotiationRoutingTest.php: No such file or directory
error: patch failed: core/phpcs.xml.dist:63
error: core/phpcs.xml.dist: patch does not apply
error: patch failed: core/tests/Drupal/Tests/Core/Routing/UrlGeneratorTest.php:299
error: core/tests/Drupal/Tests/Core/Routing/UrlGeneratorTest.php: patch does not apply
rasikap’s picture

Assigned: Unassigned » rasikap
rasikap’s picture

Assigned: rasikap » Unassigned
Status: Needs work » Needs review
FileSize
151.06 KB

Added a rerolled patch.

Status: Needs review » Needs work

The last submitted patch, 13: 2719649-13.patch, failed testing.

rasikap’s picture

Assigned: Unassigned » rasikap
rasikap’s picture

Assigned: rasikap » Unassigned
Status: Needs work » Needs review
FileSize
151 KB
crazyrohila’s picture

Status: Needs review » Needs work

I think this patch also includes spaceAfter fixes too which belong to other issue (https://www.drupal.org/node/2572659).

eg.

FILE: ....2-git/core/tests/Drupal/Tests/Core/Routing/UrlGeneratorTest.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 6 WARNINGS AFFECTING 6 LINES
----------------------------------------------------------------------
 200 | WARNING | [x] There must be no blank line following an inline
     |         |     comment

The above standard is fixed in above patch which should be part of other meta issue.

rasikap’s picture

Assigned: Unassigned » rasikap

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

rasikap’s picture

@crazyrohila do I need to remove the extra fixes from the above patch?

hitesh-jain’s picture

Assigned: rasikap » hitesh-jain
hitesh-jain’s picture

FileSize
151 KB

Rerolled against 8.3.x.

hitesh-jain’s picture

Status: Needs work » Needs review
FileSize
151 KB

Status: Needs review » Needs work

The last submitted patch, 23: fix-2719649-22.patch, failed testing.

hitesh-jain’s picture

Assigned: hitesh-jain » Unassigned
jofitz’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
149.82 KB

Re-roll.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.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.4.x-dev » 8.5.x-dev

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

mfernea’s picture

Status: Needs review » Needs work

Modifications to the phpcs.xml.dist are not correct.

harsha012’s picture

Status: Needs work » Needs review
FileSize
150.06 KB

fixed the phpcs.xml.dist and re-rolled to 8.5.x

Status: Needs review » Needs work

The last submitted patch, 30: 2719649-30.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

harsha012’s picture

Status: Needs work » Needs review
FileSize
27.76 KB
cburschka’s picture

What is the difference between #2715485: Fix 'Drupal.Commenting.InlineComment.NoSpaceBefore' coding standard and this?

Edit: Answer:

This is a NoSpaceBefore violation:

function helloWorld() {
  //prints "hello world"
  echo("Hello world");
}

and this is a SpacingBefore violation:

function helloWorld() {
// prints "hello world"
  echo("Hello world");
}
mfernea’s picture

Status: Needs review » Needs work

The modifications to phpcs.xml.dist should be:

  <rule ref="../vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/Commenting/InlineCommentSniff.php">
    <!-- Sniff for: SpacingBefore -->
    <exclude name="Drupal.Commenting.InlineComment.DocBlock"/>
    <exclude name="Drupal.Commenting.InlineComment.InvalidEndChar"/>
    <exclude name="Drupal.Commenting.InlineComment.NoSpaceBefore"/>
    <exclude name="Drupal.Commenting.InlineComment.NotCapital"/>
    <exclude name="Drupal.Commenting.InlineComment.SpacingAfter"/>
    <exclude name="Drupal.Commenting.InlineComment.WrongStyle"/>
  </rule>

Doing so, we are not overlapping with #2715485: Fix 'Drupal.Commenting.InlineComment.NoSpaceBefore' coding standard, although we'll have merge conflicts for phpcs.xml.dist, which is expected.
So, please make sure that the changes in the patch are only fixing SpacingBefore issues and nothing else.

harsha012’s picture

Status: Needs work » Needs review
FileSize
52.23 KB
21.89 KB

@mfernea

fixed as per your comment.

mfernea’s picture

Status: Needs review » Needs work

Modifications to phpcs.xml.dist look good.
But if I look at the patch I see modifications for NoSpaceBefore sniff (eg Diff.php or DiffEngine.php). It should not. The patch should only contain fixes for SpacingBefore sniff.

cburschka’s picture

Status: Needs work » Needs review
FileSize
14.84 KB
65.09 KB

This might be the right changeset

Edit: wait, no it messed up a bunch of indentation; sorry.

cburschka’s picture

Status: Needs review » Needs work
+++ b/core/modules/node/tests/src/Functional/NodeAccessLanguageAwareTest.php
+++ b/core/modules/node/tests/src/Functional/NodeAccessLanguageAwareTest.php
@@ -88,12 +88,12 @@ protected function setUp() {

@@ -88,12 +88,12 @@ protected function setUp() {
 
     // Create six nodes:
     // 1. Four Hungarian nodes with Catalan translations
-    //   - One with neither language marked as private.
+    //    - One with neither language marked as private.
     //   - One with only the Hungarian translation private.
     //   - One with only the Catalan translation private.
     //   - One with both the Hungarian and Catalan translations private.
     // 2. Two nodes with no language specified.
-    //   - One public.
+    //    - One public.
     //   - One private.
     $this->nodes['both_public'] = $node = $this->drupalCreateNode([
       'body' => [[]],

Seems to add the wrong indentation in this comment - or incompletely add the correct one.

harsha012’s picture

Status: Needs work » Needs review
FileSize
121.7 KB
mfernea’s picture

Status: Needs review » Needs work

The tests results show that there are still some cs issues.

harsha012’s picture

Status: Needs work » Needs review
FileSize
124.25 KB
8.51 KB

@mfernea

in core/tests/Drupal/Tests/Component/Graph/GraphTest.php

there is graph diagram in the comment section do we need to fix it.

Status: Needs review » Needs work

The last submitted patch, 41: 2719649-41.patch, failed testing. View results

harsha012’s picture

Status: Needs work » Needs review
FileSize
95.78 KB
harsha012’s picture

FileSize
96.41 KB
mfernea’s picture

Status: Needs review » Needs work

Not really sure what to do with the comment in core/tests/Drupal/Tests/Component/Graph/GraphTest.php which is failing the test. A quickfix would be to add // @code and // @endcode. It looks like the Drupal CS doesn't raise an issue anymore, but @code and @endcode are meant for docblocks and not inline comments. And I think we have to use this for all other inline commented code in this patch.

Suggestions are welcome.

mfernea’s picture

Drupal CS allows @code @endcode in inline comments and does that on purpose, it's not just a coincidence.
So whenever we have to deal with commented code or comments like those in GraphTest.php it's best to use these tags.
Also please pay attention to // @todo vs // @todo: or other variations. Only the first one is correct and allows the next lines to be indented with 2 spaces.

jofitz’s picture

Status: Needs work » Needs review
FileSize
97.11 KB

If I understand correctly, in #46 you are suggesting the troublesome comment in GraphTest.php should be surrounded by @code ... @endcode so that is what I have done (and re-rolled because the patch no longer applies cleanly, hence no interdiff).

mfernea’s picture

Status: Needs review » Needs work

// @todo vs // @todo: issue is not solved yet.
We should use @code @endcode tags whenever we deal with code like comments (eg ConfigImporter.php).

jofitz’s picture

Status: Needs work » Needs review
FileSize
75.64 KB
163.54 KB

Replaced all occurrences of // @todo: with // @todo.

mfernea’s picture

Status: Needs review » Needs work

If we use // @todo we can have a 2 space indent for the lines below.
We still have to fix the code like comments using @code and @endcode.

mfernea’s picture

Status: Needs work » Postponed
mfernea’s picture

Status: Postponed » Needs work
Malevi4’s picture

Small fix

diff --git a/core/modules/simpletest/src/TestDiscovery.php b/core/modules/simpletest/src/TestDiscovery.php
index 9e6c32c327..3b434eb173 100644
--- a/core/modules/simpletest/src/TestDiscovery.php
+++ b/core/modules/simpletest/src/TestDiscovery.php
@@ -193,7 +193,7 @@ public function getTestClasses($extension = NULL, array $types = []) {
       // unavailable modules. TestDiscovery should not filter out module
       // requirements for PHPUnit-based test classes.
       // @todo Move this behavior to \Drupal\simpletest\TestBase so tests can be
-      //       marked as skipped, instead.
+      // marked as skipped, instead.
       // @see https://www.drupal.org/node/1273478
       if ($info['type'] == 'Simpletest') {
         if (!empty($info['requires']['module'])) {
mfernea’s picture

https://www.drupal.org/docs/develop/coding-standards/api-documentation-a...

Prefered style:

// @todo Move this behavior to \Drupal\simpletest\TestBase so tests can be
//   marked as skipped, instead.

Still passes CS, but less nice:

// @todo Move this behavior to \Drupal\simpletest\TestBase so tests can be
// marked as skipped, instead.

Doesn't pass CS:

// @todo Move this behavior to \Drupal\simpletest\TestBase so tests can be
//       marked as skipped, instead.
harsha012’s picture

Added patch

harsha012’s picture

Status: Needs work » Needs review
jofitz’s picture

@harsha012 Can you give us more details about your patch please? What changes have you made since the previous patch in #49?

It is also helpful if you can provide an interdiff when you upload a patch, if possible. That makes it far easier to review.

I am surprised to see that your patch is about a third of the size of the previous patch. This might suggest you have missed something.

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.

klausi’s picture

Status: Needs review » Needs work
  1. +++ b/core/lib/Drupal/Core/Render/RenderCache.php
    @@ -121,9 +121,9 @@ public function set(array &$elements, array $pre_bubbling_elements) {
    -      //   '#cache_redirect' => TRUE,
    +      // '#cache_redirect' => TRUE,
    

    this break the formatting of a code example. I think we should wrap this with @code and @endcode comments, then it should not complain? Can you test if that works?

  2. +++ b/core/tests/Drupal/FunctionalTests/Update/UpdatePathTestBase.php
    @@ -416,7 +416,7 @@ protected function runDbTasks() {
         // @todo: Saving the account before the update is problematic.
    -    //   https://www.drupal.org/node/2560237
    +    // https://www.drupal.org/node/2560237
    

    do we need to fix this? I think it should work under the @todo like it is.

I think a lot of cases here could be improved with @code tags then Coder should not complain.

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.

nikitagupta’s picture

Assigned: Unassigned » nikitagupta
nikitagupta’s picture

Status: Needs work » Needs review
FileSize
18.28 KB
nikitagupta’s picture

klausi’s picture

Status: Needs review » Needs work

The change in phpcs.xml.dist is missing?

nikitagupta’s picture

Assigned: nikitagupta » Unassigned
Status: Needs work » Needs review
FileSize
36.93 KB

Reroll the patch.
@klausi the required changes for phpcs.xml.dist is already added in drupal 9.

cburschka’s picture

9.1.x phpcs.xml.dist still excludes the Drupal.Commenting.InlineComment.SpacingBefore rule:

  <rule ref="Drupal.Commenting.InlineComment">
    <!-- Sniff for: NoSpaceBefore, WrongStyle -->
    <exclude name="Drupal.Commenting.InlineComment.DocBlock"/>
    <exclude name="Drupal.Commenting.InlineComment.InvalidEndChar"/>
    <exclude name="Drupal.Commenting.InlineComment.NotCapital"/>
    <exclude name="Drupal.Commenting.InlineComment.SpacingAfter"/>
    <exclude name="Drupal.Commenting.InlineComment.SpacingBefore"/>
  </rule>
klausi’s picture

Status: Needs review » Needs work

Yep, please remove the exclude in phpcs.xml.dist.

cburschka’s picture

There's a few validations that feel overly strict. For example, in some places "Note:" is used to start a continuation indent, but the sniff does not allow this.

    // Note: this cannot be checked in ::supportsDenormalization() because at
    //       that time we only have the field item class. ::hasSerializeColumn()
    //       must be able to call $field_item->schema(), which requires a field
    //       storage definition. To determine that, the entity type and bundle
    //       must be known, which is contextual information that the Symfony
    //       serializer does not pass to ::supportsDenormalization().
 

In another place, it's @see:

    // Double open angle brackets.
    // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Double_open_angle_brackets
    // @see http://ha.ckers.org/blog/20060611/hotbot-xss-vulnerability/ to
    //      understand why this is a vulnerability.
    $data[] = ['<iframe src=http://ha.ckers.org/scriptlet.html <', '<iframe src="http://ha.ckers.org/scriptlet.html">'];

Also in a ton of places, a @todo is marked as an invalid continuation indent because it is written as "@todo: ..." rather than "@todo ..." - the colon causes the sniffer to not recognize it. That does seem like something we want to be consistent on, but I'm not sure if removing the colon from all of those is within the scope of this issue.

cburschka’s picture

Status: Needs work » Needs review
FileSize
65.62 KB
48.82 KB

Did a new pass.

This one seems to satisfy the sniff, and fixes some of the changes from the previous patch (eg indentation stripped instead of adding @code/@endcode). There were also some non-SpacingBefore fixes in the previous patch.

I fixed the "@todo:" issue by removing the colon from all @todo items that got flagged by the sniff, and changed @TODO and @fixme to @todo in a few cases.

But there are lots and lots more of those that didn't get flagged because they're single line. If we want to apply a consistent rule to those, that's definitely a separate issue.

cburschka’s picture

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.

longwave’s picture

FileSize
66.23 KB

Rerolled and fixed two more cases.

quietone’s picture

Needed a reroll

Spokje made their first commit to this issue’s fork.

Spokje’s picture

Status: Needs review » Needs work

I've put 2719649-76.patch in to an Merge Request, just for easier commenting on each change.

Feel free to post your answer as a patch and/or in the MR.

quietone’s picture

Am I right that the unresolved issues are about the link in the todos?
The example in the standard does not include 'see' or '@see' for @todos.

* @todo Remove this in D8. http://www.drupal.org/node/1234567
* @todo Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam
*   nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed
*   diam voluptua.
Spokje’s picture

@quiteone

The example in the standard does not include 'see' or '@see' for @todos.

Ah, you're absolutely right. Resolved all my comments that were about those @sees.

Only three unresolved issues left. :)

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.

Spokje’s picture

Status: Needs work » Needs review

Rebased MR on 9.3.x.

Spokje’s picture

Hiding old 9.2.x patches

longwave’s picture

Status: Needs review » Needs work

A few minor comments but this is looking good overall.

Spokje’s picture

Status: Needs work » Needs review

Thanks @longwave and his eagle eyes.

longwave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, all points addressed, this is ready to go.

  • catch committed 33ba458 on 9.3.x
    Issue #2719649 by Spokje, harsha012, jofitz, vprocessor, nikitagupta,...

  • catch committed 9a85542 on 9.2.x
    Issue #2719649 by Spokje, harsha012, jofitz, vprocessor, nikitagupta,...
catch’s picture

Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.3.x, thanks! Also committed the same patch, minus the phpcs.dist.xml change to 9.2.x.

This one is borderline for what might need to be scheduled in a beta/rc, but it's a tangible documentation as well as code style improvement.

Status: Fixed » Closed (fixed)

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