Problem/Motivation

Enable 'Drupal.Commenting.DocComment.MissingShort' coding standard.

Proposed resolution

Step 1: Remove the following line from core/phpcs.xml.dist and run composer run phpcs -- -p to get the report of what to be fixed. Also you can checkout https://www.drupal.org/pift-ci-job/1631109 from #4

<exclude name="Drupal.Commenting.DocComment.MissingShort"/>

Step 2: Fix all of them manually, as this one CAN NOT be fixed by phpcbf automatically.

Remaining tasks

Implement for

Completed

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#10 3123062-10-coder-class-missing-short-description.patch790 bytesjungle
#4 3123062-4-test-only.patch533 bytesjungle

Comments

jungle created an issue. See original summary.

foxtrotcharlie’s picture

foxtrotcharlie commented
Assigned: Unassigned » foxtrotcharlie
foxtrotcharlie’s picture

foxtrotcharlie commented

@jungle - Codesniffer does detect a number of errors with ../vendor/bin/phpcs -p but after I run the last command ../vendor/bin/phpcbf I am told "No fixable errors were found". Any idea what I need to do to get it to do the automatic fixing? Or how I can figure out why the fixer is not fixing the errors that codesniffer detects?

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
StatusFileSize
new 3123062-4-test-only.patch533 bytes

Hi, @foxtrotcharlie, thank you for working on this. Unfortunately, This one can not be fixed by phpcbf automatically.

You can remove the following line from core/phpcs.xml.dist and run composer run phpcs -- -p to get the report.

<exclude name="Drupal.Commenting.DocComment.MissingShort"/>

Meanwhile, I am submitting a patch by removing the above line to let CI tell what files need to be fixed.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Issue summary: View changes

Update IS

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Issue summary: View changes

Correct IS and adjust the format of IS

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.

foxtrotcharlie’s picture

foxtrotcharlie commented
Assigned: foxtrotcharlie » Unassigned

I'm just not going to have time to get to this, particularly because there are so many manual fixes required (1338) and my work workload has just increased dramatically. Will let it go for someone else to take on, sorry!

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

I don't think 1338 fixes in a single issue will be reviewable, especially as each of these needs a line of documentation to be written and agreed on. Should we break this down into sub-issues? How would we do that?

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
StatusFileSize
new 3123062-10-coder-class-missing-short-description.patch790 bytes

#8, no worries, thanks!

#9 yes, totally agree!

--- a/core/modules/block_content/tests/src/Functional/Hal/BlockContentHalJsonAnonTest.php
+++ b/core/modules/block_content/tests/src/Functional/Hal/BlockContentHalJsonAnonTest.php
@@ -8,6 +8,8 @@
 use Drupal\Tests\rest\Functional\AnonResourceTestTrait;
 
 /**
+ * Class BlockContentHalJsonAnonTest.
+ *
  * @group hal
  */
 class BlockContentHalJsonAnonTest extends BlockContentResourceTestBase {

I think the above kind of violations could be auto-fixed, as the short description could be Class THE_CALSS_NAME.. I made a quick and small patch to coder to identify this pattern -- class missing short description, see the patch attached. With it, 464 fixable violations found, that's amazing.

Meanwhile, I found that 734 files are *Test.php files and 1012 .php files

  1. So the first sub issue could be "Fixing class missing short desctiption"
  2. Next, identify auto-fixible patterns as possible and file new issues per pattern
  3. Further more, split by a). *Test.php, b) Not*Test.php x 1) method missing short description, 2) property missing short description, 3) function missing description (no sure if there are any) we can file 4 or 6 sub issues.
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

The first child issue #3136779: Fix DocComment.ClassMissingShort in non tests is ready for review now. Down to 510 violations left from 1066 in the branch 9.1.x.

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.

quietone’s picture

quietone commented
Issue tags: +Coding standards

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.

scottatdrake’s picture

scottatdrake commented

I started looking at this issue at #drupalconPortland2024. My understanding of the blocker here is that adding comments for 1,300+ classes in the format "Class [class name]" is not valuable, and there are too many to write manually. In fact, I think there may be a PHPStan rule prohibiting repeating the class name in the comment, though I'd need to verify that claim.

A couple of years have passed since this ticket was last updated. Now, we have LLM's like ChatGPT which can be pretty good at summarizing code. My idea is to write a script of some sort that passes the work of writing a meaningful description to one of them.

Reviewing such a large MR will still be a challenge but we may be able to automate the writing portion.

I'll wait for feedback before pursing this approach further.

quietone’s picture

quietone commented
Parent issue: #2571965: [meta] Fix PHP coding standards in core, stage 1 » #3464117: [meta] Fix 'Drupal.Commenting.DocComment' coding standard
quietone’s picture

quietone commented
Title: Fix 'Drupal.Commenting.DocComment.MissingShort' coding standard » [meta] Fix 'Drupal.Commenting.DocComment.MissingShort' coding standard
Issue summary: View changes

There are too many changes here and they will require some thought to decide on a correct summary line, so making this a meta.

quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.