Review of the 8.x-1.x branch (commit 295921f):
Your README.md does not follow best practices (headings need to be uppercase). See https://www.drupal.org/node/2181737 .
The INTRODUCTION section is missing.
The REQUIREMENTS section is missing.
The INSTALLATION section is missing.
The CONFIGURATION section is missing.
Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
DrupalPractice has found some issues with your code, but could be false positives.
FILE: ...reviewsh/pareview_temp/src/Controller/HoneypotSettingsController.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
225 | WARNING | NodeType::loadMultiple calls should be avoided in
| | classes, use dependency injection instead
--------------------------------------------------------------------------
Time: 1.47 secs; Memory: 6Mb
This automated report was generated with PAReview.sh, your friendly project application review script.
FILE: ...000000/site1101/web/vendor/drupal/pareviewsh/pareview_temp/README.md
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 9 WARNINGS AFFECTING 9 LINES
--------------------------------------------------------------------------
4 | WARNING | Line exceeds 80 characters; contains 138 characters
43 | WARNING | Line exceeds 80 characters; contains 153 characters
45 | WARNING | Line exceeds 80 characters; contains 83 characters
46 | WARNING | Line exceeds 80 characters; contains 93 characters
57 | WARNING | Line exceeds 80 characters; contains 87 characters
63 | WARNING | Line exceeds 80 characters; contains 186 characters
66 | WARNING | Line exceeds 80 characters; contains 112 characters
70 | WARNING | Line exceeds 80 characters; contains 81 characters
71 | WARNING | Line exceeds 80 characters; contains 90 characters
--------------------------------------------------------------------------
FILE: ...site1101/web/vendor/drupal/pareviewsh/pareview_temp/honeypot.install
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
9 | WARNING | [x] Unused use statement
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------
FILE: .../site1101/web/vendor/drupal/pareviewsh/pareview_temp/honeypot.module
--------------------------------------------------------------------------
FOUND 5 ERRORS AND 1 WARNING AFFECTING 4 LINES
--------------------------------------------------------------------------
26 | WARNING | [ ] Avoid backslash escaping in translatable strings
| | when possible, use "" quotes instead
144 | ERROR | [ ] Key specified for array entry; first entry has no
| | key
172 | ERROR | [x] Expected 1 space before "*"; 0 found
172 | ERROR | [x] Expected 1 space after "*"; 0 found
243 | ERROR | [x] Expected 1 space before "*"; 0 found
243 | ERROR | [x] Expected 1 space after "*"; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------
Time: 2.2 secs; Memory: 6Mb
Comment | File | Size | Author |
---|---|---|---|
#4 | 3106521-4-coding-standards.patch | 3.63 KB | geerlingguy |
|
Comments
Comment #2
geerlingguy CreditAttribution: geerlingguy at Midwestern Mac, LLC commentedFor the README, I disagree with the phpcs standards for Drupal and have dismissed those warnings in the past. It's much harder to maintain a readable README file when it follows the letter of the law there.
For the other issues, I hadn't seen them come up in the past few runs, maybe they're new issues?
Comment #3
geerlingguy CreditAttribution: geerlingguy at Midwestern Mac, LLC commentedThese issues look like they're also popping up in the daily automated test runs: https://www.drupal.org/pift-ci-job/1507235
honeypot.install
honeypot.module
Comment #4
geerlingguy CreditAttribution: geerlingguy at Midwestern Mac, LLC commentedFor the 'Key specified for array entry; first entry has no key' warning, I think we might be able to follow the approach of issue #2908266: Fix 'Squiz.Arrays.ArrayDeclaration.NoKeySpecified' and 'Squiz.Arrays.ArrayDeclaration.KeySpecified' coding standard to fix it (just add a 0 index to the
form_element
item), and for that and the rest, see attached patch.Comment #6
geerlingguy CreditAttribution: geerlingguy at Midwestern Mac, LLC commentedI'm choosing to ignore the CS issues in the README file, as the line length rule is highly annoying to me :)
Comment #7
ankushgautam76@gmail.comYes it is fixed.