Problem/Motivation

The changes required for 'Drupal.Commenting.DocComment.ShortSingleLine' are too large and need to be split into child issues.

Proposed resolution

Fix class property comments for sniff 'Drupal.Commenting.DocComment.ShortSingleLine'.

Remaining tasks

Patch
Review - Instructions for adding the sniff and running the test are in the Issue Summary of #3123060: Enable 'Drupal.Commenting.DocComment.ShortSingleLine' coding standard
Commit

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

quietone’s picture

Status: Active » Needs review
FileSize
33.27 KB
quietone’s picture

TR’s picture

Somehow testing never got triggered on the patch in #3, so I triggered it manually.

quietone’s picture

Issue tags: +beta blocker
quietone’s picture

Issue tags: -beta blocker +beta target

Wrong tag.

bbrala’s picture

Status: Needs review » Needs work

Ok, went through all the results and found a few missing changes:



FILE: /home/localcopy/drupal-core-10-0-x/web/core/modules/views/src/Plugin/views/argument/ArgumentPluginBase.php
-------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 10 ERRORS AFFECTING 10 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------------------
   76 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
      |       | (Drupal.Commenting.DocComment.ShortSingleLine)
-------------------------------------------------------------------------------------------------------------------------------------------------------------

-------------------------------------------------------------------------------------------------------------------------------------------------


FILE: /home/localcopy/drupal-core-10-0-x/web/core/lib/Drupal/Core/Form/FormState.php
-------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------------------
  97 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
     |       | (Drupal.Commenting.DocComment.ShortSingleLine)
 110 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
     |       | (Drupal.Commenting.DocComment.ShortSingleLine)
 127 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
     |       | (Drupal.Commenting.DocComment.ShortSingleLine)
 198 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
     |       | (Drupal.Commenting.DocComment.ShortSingleLine)
-------------------------------------------------------------------------------------------------------------------------------------------------------------

Other than those this change looks good.

quietone’s picture

Status: Needs work » Needs review
FileSize
5 KB
35.64 KB

@bbrala, thanks for the review!

The patch needed a reroll and I did both the reroll and the changes for #7 in this patch. The changes for the reroll were in \Drupal\views\Plugin\views\query\Sql.

bbrala’s picture

Could you please attach an interdiff, not a diff, kinda unreadable and means there might be parts missing.

bbrala’s picture

If you want to know why, we have great docs on that :)

bbrala’s picture

Status: Needs review » Reviewed & tested by the community

I've went through the new changes, those look good. Interdiff is failing, probably because of the rebase. But the diff makes sense.

Setting to RTBC :) Thanks for makin this happen.

quietone’s picture

FileSize
11.54 KB
2.33 KB

Thanks!

Yes, and interdiff is preferred. However, that often fails with a rerolll. When rerolling and the interdiff has failures I create a diff. Although I don't usually use the '-u' option as specified in the instructions as What about rerolled patches?. I have done that now and am uploading a new diff.

I am also uploading the interdiff which has a failure.

Cheers ;-)

quietone’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
630 bytes
35.63 KB

The patch applies to 10.1.x but not 9.5.x. Here is a patch for 9.5.x.

bbrala’s picture

The change is really minimal. Inteerdiff is looking good, patch at those files also looks good. Therefor i will RTBC if testrun is green.

bbrala’s picture

Status: Needs review » Reviewed & tested by the community

Rtbc. Just to note, my inspection was visual since the dif was so minimal.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 13: 3268829-13-9.5.x.patch, failed testing. View results

quietone’s picture

Test fail seems unrelated to these cs fixes, rerunning tests.

1) Drupal\Tests\views_ui\FunctionalJavascript\LibraryCachingTest::testConsecutiveDialogRequests
WebDriver\Exception\CurlExec: Curl error thrown for http POST to http://chromedriver-jenkins-drupal-patches-150606:9515/session/c8e259ea0f144a1e741a96e2e210e771/moveto with params: {"element":"0.22354884482981108-1"}
quietone’s picture

Status: Needs work » Needs review

Tests passing, back to NR

quietone’s picture

Status: Needs review » Reviewed & tested by the community

Oops, that should have been back to RTBC.

alexpott’s picture

Version: 10.0.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 5b84c09246 to 10.1.x and d6f7236a17 to 10.0.x. Thanks!
Committed db0a7b4 and pushed to 9.5.x. Thanks!

Backported to 9.5.x as this is a documentation only patch.

  • alexpott committed 5b84c09 on 10.1.x
    Issue #3268829 by quietone, bbrala: Fix class property comments for...

  • alexpott committed d6f7236 on 10.0.x
    Issue #3268829 by quietone, bbrala: Fix class property comments for...

  • alexpott committed db0a7b4 on 9.5.x
    Issue #3268829 by quietone, bbrala: Fix class property comments for...

Status: Fixed » Closed (fixed)

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