This is a part of the attempt to fix #2572645: [Meta] Fix 'Drupal.Commenting.FunctionComment' coding standard
This issue is created to tackle the sub-sniffs
Drupal.Commenting.FunctionComment.InvalidReturn
Drupal.Commenting.FunctionComment.VoidReturn
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | interdiff-2722621-10-12.txt | 882 bytes | anoopjohn |
| #12 | drupal-coding-standards-function-comment-InvalidReturn-VoidReturn-2722621-12.patch | 5.82 KB | anoopjohn |
Issue fork drupal-2722621
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
anoopjohn commentedPlease find attached a patch for the same
Comment #4
anoopjohn commentedNot sure why the patch failed testing. I just downloaded and tested against latest head and it was working. Marking for review again.
Comment #5
mile23Re-running test.
Comment #6
mile23I don't think we need to add these notes. Also, strictly speaking, all methods return NULL, which isn't the same as 'nothing.' :-)
Comment #7
anoopjohn commentedThanks for the review Mile23. I have updated the patch as per your recommendation. Please find attached the revised patch and the interdiff.
Comment #8
anoopjohn commentedComment #9
mile23Needs a reroll after changes from #2721909: Fix Drupal.Commenting.FunctionComment.ReturnCommentIndentation
Comment #10
anoopjohn commentedThanks again for the review Mile23. Please find attached the rerolled patch. I was wondering if it would be alright to give two patches - one for the phpcs change and the other with the remaining? The phpcs one which is the one which fails all the time and it is probably easier to delete the line than go through the re-roll process which does not add any value.
Comment #11
mile23Not sure what you mean about the two patches...
Current patch gives me this:
Also:
We don't need to say that it doesn't return anything.
Comment #12
anoopjohn commentedThanks for the review @Mile23
I meant creating two patches for such fixes related to each coding standards issue. One patch with the change in phpcs.xml.dist and the other patch with changes in the rest of the files. The rerolling is usually only required for the phpcs.xml.dist part of the patch
Please find attached the revised patch and the interdiff.
Comment #13
anoopjohn commentedComment #14
klausiLooks good, verified manually that a run of phpcs does not throw any errors with the changed phpcs.xml.dist file.
Comment #15
alexpottCommitted 5f925ef and pushed to 8.1.x and 8.2.x. Thanks!
Fixed on 8.1.x during commit.
Comment #16
alexpott