Problem/Motivation
Running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig gives the following warnings/errors.
FILE: ./quicklink_2.0.x/tests/quicklink_test/quicklink_test.info.yml
1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
7 | WARNING | All dependencies must be prefixed with the project name, for example "drupal:"
FILE: ./quicklink_2.0.x/quicklink.install
8 | ERROR | [x] Line indented incorrectly; expected 0 spaces, found 1
FILE: ./quicklink_2.0.x/quicklink.module
141 | ERROR | The array declaration extends to column 150 (the limit is 80). The array content should be split up over multiple
| | lines
FILE: ./quicklink_2.0.x/src/Form/QuicklinkConfigForm.php
62 | ERROR | [x] Missing function doc comment
62 | ERROR | [ ] Public method name "QuicklinkConfigForm::quicklink_config_form_validate" is not in lowerCamel format
63 | ERROR | [x] Short array syntax must be used to define arrays
73 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
73 | ERROR | [x] Short array syntax must be used to define arrays
206 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
207 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
212 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
213 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
223 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
224 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
231 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
232 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
233 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
240 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
241 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
242 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
248 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
314 | ERROR | [x] Short array syntax must be used to define arrays
324 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
324 | ERROR | [x] Short array syntax must be used to define arrays
FILE: ./quicklink_2.0.x/README.md
76 | WARNING | Line exceeds 80 characters; contains 81 characters
Steps to reproduce
Proposed resolution
Those errors/warnings need to be fixed.
Remaining tasks
We can remove quicklink_config_form_validate function from src/Form/QuicklinkConfigForm.php file, looks like not used any where.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | Screenshot from 2024-06-13 12-04-44.png | 37.34 KB | silvi.addweb |
| #8 | phpcs-fixes-3329641-8.patch | 7.84 KB | nisha_j |
| #2 | 3329641-2.patch | 7.76 KB | samitk |
Issue fork quicklink-3329641
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
samitk commentedError/warnings are fixed.
Comment #3
charchil khandelwal commentedI will review this.
Comment #5
charchil khandelwal commentedThanks @samit.310@gmail.com, patch #2 applied cleanly, all errors and warnings are fixed.
And i also created MR for same.
Please review.
Comment #6
avpadernoLines showing commands should not be split on two or more lines or be formatted to make clear the command is only one.
That description is still repeating the class name instead of describing what the class does.
Also, the report does not show any warning/error for that line, which means that either the report is not complete, or the patch is changing files that should not change. (I take it is the first, since Class QuicklinkConfig. is not describing what the class does.)
There is no need to use
@codingStandardsIgnoreStart.The Quicklink module (this module) is not a Drupal core module.
Comment #7
avpadernoComment #8
nisha_j commentedComment #11
anita verma commentedHello
Updated the suggested changes as per #6 .
Kindly review the changes.
Thankyou.
Comment #12
silvi.addweb commentedHello,
After checking, I discovered one more error.
Comment #15
avpadernoIt seems part of the changes done in this issue has been already committed. The only PHP_CodeSniffer error reported by GitLab CI is the following one.
FILE: ...9641/web/modules/custom/quicklink-3329641/src/Form/QuicklinkConfigForm.php -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 65 | ERROR | Public method name | | "QuicklinkConfigForm::quicklink_config_form_validate" is not in | | lowerCamel format | | (Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps) -------------------------------------------------------------------------------- Time: 237ms; Memory: 6MBComment #16
avpadernoTruly, that method can be removed: It is not used, and it duplicates
validateForm(). (Its code has not been fully converted to Drupal 8+ too.)Comment #17
avpadernoComment #18
clarkssquared commentedHi
I applied the updated MR !9 and I can confirm that it fixes PHPCS issues
retaining the status to needs review for other's feedback