Problem/Motivation
Getting following error/warnings.
FILE: /var/www/html/modules/contrib/printjs/printjs.module
-----------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------
1 | ERROR | [x] Missing file doc comment
6 | ERROR | [x] Namespaced classes/interfaces/traits should be referenced with use statements
-----------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------
FILE: /var/www/html/modules/contrib/printjs/printjs.info.yml
-------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------
1 | WARNING | Remove "project" from the info file, it will be added by drupal.org packaging automatically
1 | WARNING | Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
-------------------------------------------------------------------------------------------------------------
FILE: /var/www/html/modules/contrib/printjs/src/Printjs.php
---------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------
22 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
---------------------------------------------------------------------------------------------
FILE: /var/www/html/modules/contrib/printjs/src/Plugin/Block/PrintJsBlock.php
---------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------
37 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
---------------------------------------------------------------------------------------------
Time: 2.53 secs; Memory: 6MB
Steps to reproduce
Run following command
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml modules/contrib/printjs/
Proposed resolution
Above errors/warnings need to be fixed.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3348615-2.patch | 4.71 KB | samitk |
Issue fork printjs-3348615
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:
- 3348615-fix-the-issues
changes, plain diff MR !3 /
changes, plain diff MR !2
- printjs_issues
changes, plain diff MR !4
Comments
Comment #2
samitk commentedAbove errors/warnings has been fixed.
Comment #3
hardikpandya commentedThe patch fixes reported phpcs issues. RTBC +1
Comment #4
avpadernoComment #5
avpadernoThat is not the description for a .module file that is normally used. It does not even make sense, as it says that printjs.module contains printjs.module.
The namespace is missing.
Probably the article should be different.
Comment #8
kunalgautam commentedComment #9
avpadernoThe usual comment is Hook implementations for [module name].
The
phpcsreport shown in the issue summary does not sayhook_help()needs to be implemented. That is a change for a different issue.The class name does not include its namespace.
Comment #11
kunalgautam commentedComment #12
avpadernoThe module name is not printjs. Do not get confused between module machine name and module name, since those are different.
Also, it is Hook implementations, since a module file generally contains more than a single hook implementation; even in the case there is just a hook, the plural is always used.
The class name and its namespace are missing from the short description.
The usual comment is "Constructs a new [class name] object."
A description like Injected service. is too generic; it would suit every service.
Comment #13
shalini_jha commentedI will work on this issue.
Comment #14
shalini_jha commentedComment #16
shalini_jha commentedComment #17
avpadernoThat is not the documentation comment used for modules. See comment #12 for the correct documentation comment, which the previous MR was almost providing.
The report in the issue summary does not say that hook must be implemented. This issue is for fixed what reported by PHP_CodeSniffer; other changes are off-topic for this issue, except in the case of lines already changed for what PHP_CodeSniffer reports.
The class name does not include its namespace.
using the provided configuration and string translation services is not necessary and it is not used for constructors documentation comments.
There are trailing spaces and an extraneous asterisk.
The class namespace is missing.
Comment #19
lazzyvn commentedPlease patch on dev version i updated
Comment #22
avpaderno