Problem/Motivation
The changes required for 'Drupal.Commenting.DocComment.ShortSingleLine' are too large and need to be split into child issues.
Proposed resolution
Fixes for sniff 'Drupal.Commenting.DocComment.ShortSingleLine' that are just adding new lines.
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 |
|---|---|---|---|
| #5 | 3268746-5.patch | 11.75 KB | quietone |
| #5 | interdiff-2-5.txt | 472 bytes | quietone |
| #2 | 3268746-2.patch | 12.53 KB | quietone |
Comments
Comment #2
quietone commentedAnd here is a patch.
Comment #3
xjmThis is the only zinger here.
{@inheritdoc}cannot coexist with any other docs. So I would split this out of the patch, and we can decide whether to delete the second line or to replace the inheritdoc with a full docblock.I also note that a lot of the one line summaries are missing the infamous "s" (the initial verb is supposed to be in the indicative third person). However, fine with scoping that to a followup since that problem likely exists many other places in core as well. So the rest of this patch seems fine once the above hunk is moved out.
Comment #4
xjmComment #5
quietone commentedAh, yes the infamous 's'. If a sniff is helpful with that I'd wait for a sniff. If not, then I would wait until all the existing sniffs are fixed. Otherwise, it feels like chasing one's tail.
Followup made for the 'zinger' #3269000: fix docblock of TestFieldWidgetMultiple::isApplicable which is removed from this patch.
Comment #6
quietone commentedComment #7
shashwat purav commentedThe patch in #5 is applied successfully to 10.0.x branch.
Comment #8
xjm@Shashwat Purav, thank you for reviewing this issue!
The automated testing infrastructure tells us whether the change set still applies, so we do not need people to review that. It is also not sufficient criteria for the issue to be marked "Reviewed and Tested by the Community".
What we do need people to review is whether the issue has a correct scope, whether it passes the core gates, whether the solution completely fixes the problem without introducing other problems, and whether it's the best solution we can come up with. See the patch review guide for more information.
When you do post a review, be sure to describe what you reviewed and how. This helps other reviewers understand why you considered the issue RTBC (and is considered for issue credit).
Also see the issue credit guidelines for more information on which kinds of contributions are credited.
Comment #9
xjmGood to go. This should be backportable all the way to 9.3.x. (We would not backport the rule, just the cleanups where they apply.)
Comment #10
xjmComment #12
quietone commentedrandom fail on Drupal\Tests\quickedit\FunctionalJavascript\QuickEditFileTest::testRemove, retesting.
restoring rtbc.
Comment #14
spokjeRandom fail (again) on
Drupal\Tests\quickedit\FunctionalJavascript\QuickEditFileTest::testRemove, re-RTBCComment #16
spokje#3268746-14: Fix missing newlines for 'Drupal.Commenting.DocComment.ShortSingleLine'
Comment #17
quietone commentedChanging the version and starting tests for 9.5.x
Comment #18
alexpottCommitted and pushed b6a3c63309 to 10.0.x and 37f6b990ef to 9.5.x and 49ce2e6d6e to 9.4.x and 7177a7cbe1 to 9.3.x. Thanks!
Backported to 9.3.x since this is only documentation.