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.
Comment | File | Size | Author |
---|---|---|---|
#12 | 2707641-2-12.patch | 23.34 KB | alexpott |
Comment | File | Size | Author |
---|---|---|---|
#12 | 2707641-2-12.patch | 23.34 KB | alexpott |
Comments
Comment #2
alexpottPatch attached enables the Drupal.Commenting.FunctionComment sniff and changes the severity of all the other sub-sniff of the rule so that phpcs passes with the patch.
The patch here is based on the @anoopjohn's work in #2700661: Ensure core compliance to Drupal.Commenting.FunctionComment.ParamCommentIndentation (part 1)
Comment #3
alexpottCleaning it up a bit.
Comment #4
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedI had earlier created this issue and had added this as a child of the part 1 issue #2707335: Ensure core compliance to Drupal.Commenting.FunctionComment.ParamCommentIndentation - Follow up issue for less simple fixes. Mark that issue as duplicate?
Comment #5
alexpott@anoopjohn ah - sorry I didn't see that issue. My fault. Let's choose this one because the comments changes are more correct. You changes to phpcs.xml.dist on the other issue are great though. So if you could bring them into this issue that'd be fantastic.
Comment #6
alexpottIn #2707335-4: Ensure core compliance to Drupal.Commenting.FunctionComment.ParamCommentIndentation - Follow up issue for less simple fixes @anoopjohn asks
imo it makes to just exclude the ones that actually need fixes - less noise and better coding standards checking for the win!
Comment #7
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedI have applied the exclusion rules as per #2707335-4: Ensure core compliance to Drupal.Commenting.FunctionComment.ParamCommentIndentation - Follow up issue for less simple fixes and have re-created the patch from #3 above. On rerunning the excluded rules I see that the number of sub sniffs that have to be excluded is more than the list I had listed along with the example shown. Not sure how I got the initial subset. I have made necessary changes and phpcs passes with the set of rules in phpcs.xml.dist now. The set of sub sniffs is still smaller than all the sub sniffs though.
I did a bit of grouping / mapping of the sniffs and sub-sniffs that fail and had created and shared a google spreadsheet with the details at #2571965-52: [meta] Fix PHP coding standards in core
Comment #9
alexpottThe test fail has nothing to do with the @anoopjohn's changes.
Comment #10
andypostOverall looks good
usually we describe properties at the class annotation in other elements
Comment #11
alexpott@andypost this is not documenting the properties of the render element it is documenting what the pre-render callback is using as and as such is exactly how all the other render elements do it. See Checkbox::preRenderCheckbox() for example.
Comment #12
alexpottrerolled.
I don't think this comment is necessary - this file is self-documenting enough.
Comment #13
andypostsubject of issue is exactly about this, why excluded?
Comment #14
alexpott@andypost nope - it is about Drupal.Commenting.FunctionComment.ParamCommentIndentation
It is excluded to make these changes manageable. Once this in we can file to remove and fix each exclusion one by one.
Comment #15
andypostThen it looks completed
Comment #16
alexpottCommitted 638db4d and pushed to 8.1.x and 8.2.x. Thanks!