Problem/Motivation
FILE: /home/addweb/Drupal8-vagrant/web/web/drupal10_10/web/modules/contrib/comment_notify/comment_notify.tokens.inc
-------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
-------------------------------------------------------------------------------------------------------------------
38 | WARNING | Unused variable $language_code.
41 | WARNING | Unused variable $language_code.
43 | WARNING | Unused variable $sanitize.
-------------------------------------------------------------------------------------------------------------------
FILE: /home/addweb/Drupal8-vagrant/web/web/drupal10_10/web/modules/contrib/comment_notify/tests/src/Functional/CommentNotifyNotificationsTest.php
-------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 7 WARNINGS AFFECTING 7 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------
70 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
75 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
76 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
154 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
166 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
228 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
257 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
-------------------------------------------------------------------------------------------------------------------------------------------------
FILE: /home/addweb/Drupal8-vagrant/web/web/drupal10_10/web/modules/contrib/comment_notify/tests/src/Functional/CommentNotifyTestBase.php
------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
------------------------------------------------------------------------------------------------------------------------------------------
101 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
114 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
------------------------------------------------------------------------------------------------------------------------------------------
FILE: /home/addweb/Drupal8-vagrant/web/web/drupal10_10/web/modules/contrib/comment_notify/tests/src/Functional/CommentNotifyUserPreferencesTest.php
---------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 25 WARNINGS AFFECTING 25 LINES
---------------------------------------------------------------------------------------------------------------------------------------------------
57 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
70 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
71 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
72 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
85 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
86 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
87 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
101 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
102 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
103 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
116 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
117 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
118 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
130 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
140 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
144 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
152 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
160 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
178 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
187 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
189 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
192 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
222 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
228 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
230 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
---------------------------------------------------------------------------------------------------------------------------------------------------
FILE: /home/addweb/Drupal8-vagrant/web/web/drupal10_10/web/modules/contrib/comment_notify/tests/src/Functional/CommentNotifyAnonymousTest.php
---------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
---------------------------------------------------------------------------------------------------------------------------------------------
43 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
74 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
130 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
---------------------------------------------------------------------------------------------------------------------------------------------
FILE: /home/addweb/Drupal8-vagrant/web/web/drupal10_10/web/modules/contrib/comment_notify/src/Form/CommentNotifyUnsubscribe.php
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------------------------------
66 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
72 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
-----------------------------------------------------------------------------------------------------------------------------------------
FILE: /home/addweb/Drupal8-vagrant/web/web/drupal10_10/web/modules/contrib/comment_notify/comment_notify.migrate.inc
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
-----------------------------------------------------------------------------------------------------------------------------------------
25 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
26 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
27 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
-----------------------------------------------------------------------------------------------------------------------------------------
FILE: /home/addweb/Drupal8-vagrant/web/web/drupal10_10/web/modules/contrib/comment_notify/comment_notify.install
----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------
109 | WARNING | Unused variable $entity_type.
----------------------------------------------------------------------------------------------------------------
FILE: /home/addweb/Drupal8-vagrant/web/web/drupal10_10/web/modules/contrib/comment_notify/comment_notify.module
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 5 WARNINGS AFFECTING 5 LINES
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
220 | WARNING | Unused variable $field_name.
230 | WARNING | Doc comment indicates hook_form_alter() but function signature is "comment_notify_form_user_form_alter" instead of "comment_notify_form_alter". Did you mean hook_form_FORM_ID_alter()?
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Steps to reproduce
~/.config/composer/vendor/squizlabs/php_codesniffer/bin/phpcs --standard=DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml /modules/contrib/comment_notify/
| Comment | File | Size | Author |
|---|---|---|---|
| comment_notify_phpcs.patch | 24.39 KB | silvi.addweb |
Issue fork comment_notify-3445084
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 #6
cleavinjosh commentedHi,
I fixed some of the phpcs issues. These are the remaining issues:
Please review.
Thank you.
Comment #7
riddhi.addweb commentedThe patch was applied cleanly and after applying the patch the errors are still not resolved.
Comment #9
vinmayiswamy commentedFixed most of the issues reported by phpcs in the same MR.
The below are the remaining issues to be fixed.
Kindly please check and advise.
Thanks!
Comment #10
gnugetCurrent CI baseline update
The current 8.x-1.x baseline still fails PHPCS as an allowed failure, so this existing issue remains the correct focused place for the coding-standards cleanup.
Exact CI findings
Pipeline 911674, PHPCS job 11344549 reports 32 errors and 2 warnings. The report lists:
comment_notify.inc(5 errors),comment_notify.migrate.inc(4),comment_notify.module(4),src/UserNotificationSettings.php(9),src/Form/CommentNotifySettings.php(1 error, 1 warning),src/Form/CommentNotifyUnsubscribe.php(1),tests/src/Functional/CommentNotifyConfigPageTest.php(3),tests/src/Functional/CommentNotifyUserPreferencesTest.php(5), andtests/src/Kernel/CommentNotifyTokenReplaceTest.php(1 warning). Violation sources are 8 null-coalesce errors; 6 missing parameter types; 6 gender-neutral-language errors; 3 missing parameter definitions; 2 missing parameter comments; and one each for class closing brace, missing return comment, parameter-name mismatch, inline-comment spacing, inline-variable-comment format, todo format, method visibility, function spacing, and repeated empty lines. PHPCBF can fix 15 violations.Proposed resolution
Refresh the existing issue fork branch from current
8.x-1.x, apply only coding-standard changes, and update the existing MR !14 rather than opening a duplicate MR.Testing instructions
AI use disclosure
AI-Generated: Yes. OpenAI Codex was used to inspect the current CI output, draft this update, and will assist with implementation and verification. All changes and test results will be reviewed by a human before merge.
Comment #11
gnugetValidation complete.
Merge request: https://git.drupalcode.org/project/comment_notify/-/merge_requests/14
Pipeline: https://git.drupalcode.org/project/comment_notify/-/pipelines/911696
Targeted PHPCS job: https://git.drupalcode.org/project/comment_notify/-/jobs/11344748 — passed.
The complete Comment Notify test directory was also run locally in DDEV and completed successfully. No merge was performed.
AI-Generated: Yes. OpenAI Codex assisted with implementation, test execution, CI inspection, and drafting this validation update. The change remains subject to human review before merge.
Comment #12
vinodhini.e commentedHi,
Tested the latest patch on Drupal 10.6.10.
The issue appears to be fixed. However, I ran PHPCS locally and found some remaining issues:
Unused variables ($language_code, $sanitize)
Filename/class name mismatch (CommentNotifyMigrationHandler.inc)
Multiple t() usage warnings in classes
Please see the PHPCS output below for details. Can these remaining issues be reviewed?
FILE: /web/modules/contrib/comment_notify/comment_notify.tokens.inc
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
------------------------------------------------------------------------------------------------------------------
38 | WARNING | Unused variable $language_code.
41 | WARNING | Unused variable $language_code.
43 | WARNING | Unused variable $sanitize.
FILE: /web/modules/contrib/comment_notify-3445084/comment_notify.migrate.inc
FOUND 1 ERROR AND 3 WARNINGS AFFECTING 4 LINES
-----------------------------------------------------------------------------------------------------------------------------------------
11 | ERROR | Filename doesn't match class name; expected file name "CommentNotifyMigrationHandler.inc"
25 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
26 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
27 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
-----------------------------------------------------------------------------------------------------------------------------------------
FILE: /web/modules/contrib/comment_notify/tests/src/Functional/CommentNotifyTestBase.php
------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
------------------------------------------------------------------------------------------------------------------------------------------
101 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
114 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
------------------------------------------------------------------------------------------------------------------------------------------
FILE: /web/modules/contrib/comment_notify/tests/src/Functional/CommentNotifyAnonymousTest.php
--------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
--------------------------------------------------------------------------------------------------------------------------------------------
43 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
74 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
131 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
--------------------------------------------------------------------------------------------------------------------------------------------
FILE: /web/modules/contrib/comment_notify/tests/src/Functional/CommentNotifyNotificationsTest.php
------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 7 WARNINGS AFFECTING 7 LINES
------------------------------------------------------------------------------------------------------------------------------------------------
72 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
79 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
80 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
189 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
201 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
263 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
292 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
------------------------------------------------------------------------------------------------------------------------------------------------
FILE: /web/modules/contrib/comment_notify/tests/src/Functional/CommentNotifyUserPreferencesTest.php
--------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 25 WARNINGS AFFECTING 25 LINES
--------------------------------------------------------------------------------------------------------------------------------------------------
58 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
71 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
72 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
73 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
86 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
87 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
88 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
102 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
103 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
104 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
117 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
118 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
119 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
131 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
141 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
145 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
153 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
161 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
179 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
188 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
190 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
193 | 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
229 | 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
--------------------------------------------------------------------------------------------------------------------------------------------------
FILE: /web/modules/contrib/comment_notify/src/Form/CommentNotifyUnsubscribe.php
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------------------------------
68 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
74 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
-----------------------------------------------------------------------------------------------------------------------------------------
Thanks.
Comment #13
gnugetThank you for testing and reporting these findings.
The main goal of this issue is to resolve the PHPCS violations reported by the project’s GitLab CI/CD configuration and make the PHPCS job pass. That CI job currently uses the Drupal standard only; it does not enable DrupalPractice.
Most of the reported warnings—unused variables and t() calls inside classes—come from DrupalPractice and are therefore outside the scope of this focused issue. The filename/class-name finding also could not be reproduced with the Coder and PHPCS configuration used by CI.
Please create a separate issue for the remaining DrupalPractice findings so they can be reviewed and addressed independently without expanding this MR’s scope.
The latest MR pipeline confirms that the CI PHPCS job passes with no coding-standard errors or warnings.
Thanks again!!!
Comment #15
gnuget