Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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
Comment | File | Size | Author |
---|---|---|---|
#13 | 3268829-13-9.5.x.patch | 35.63 KB | quietone |
#13 | interdiff-8-13-9.5.x.txt | 630 bytes | quietone |
#12 | interdiff-3-8.txt | 2.33 KB | quietone |
#12 | diff-3-8-v2.txt | 11.54 KB | quietone |
#8 | 3268829-8.patch | 35.64 KB | quietone |
|
Comments
Comment #2
quietone CreditAttribution: quietone at PreviousNext commentedComment #3
quietone CreditAttribution: quietone at PreviousNext commentedMove property changes from #3268833: Fix method comments in tests for Drupal.Commenting.DocComment.ShortSingleLine to here.
Comment #4
TR CreditAttribution: TR commentedSomehow testing never got triggered on the patch in #3, so I triggered it manually.
Comment #5
quietone CreditAttribution: quietone at PreviousNext commentedComment #6
quietone CreditAttribution: quietone at PreviousNext commentedWrong tag.
Comment #7
bbralaOk, went through all the results and found a few missing changes:
Other than those this change looks good.
Comment #8
quietone CreditAttribution: quietone at PreviousNext commented@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.
Comment #9
bbralaCould you please attach an interdiff, not a diff, kinda unreadable and means there might be parts missing.
Comment #10
bbralaIf you want to know why, we have great docs on that :)
Comment #11
bbralaI'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.
Comment #12
quietone CreditAttribution: quietone at PreviousNext commentedThanks!
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 ;-)
Comment #13
quietone CreditAttribution: quietone at PreviousNext commentedThe patch applies to 10.1.x but not 9.5.x. Here is a patch for 9.5.x.
Comment #14
bbralaThe change is really minimal. Inteerdiff is looking good, patch at those files also looks good. Therefor i will RTBC if testrun is green.
Comment #15
bbralaRtbc. Just to note, my inspection was visual since the dif was so minimal.
Comment #17
quietone CreditAttribution: quietone at PreviousNext commentedTest fail seems unrelated to these cs fixes, rerunning tests.
Comment #18
quietone CreditAttribution: quietone at PreviousNext commentedTests passing, back to NR
Comment #19
quietone CreditAttribution: quietone at PreviousNext commentedOops, that should have been back to RTBC.
Comment #20
alexpottCommitted 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.