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/

CommentFileSizeAuthor
comment_notify_phpcs.patch24.39 KBsilvi.addweb
Command icon 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

silvi.addweb created an issue. See original summary.

Status: Needs review » Needs work

The last submitted patch, comment_notify_phpcs.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

nilesh.addweb made their first commit to this issue’s fork.

cleavinjosh made their first commit to this issue’s fork.

cleavinjosh’s picture

Status: Needs work » Needs review

Hi,

I fixed some of the phpcs issues. These are the remaining issues:

➜  contrib phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml comment_notify

FILE: /Users/interns/Demo-site/drupal-org-issues/web/modules/contrib/comment_notify/comment_notify.install
----------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------
 109 | WARNING | Unused variable $entity_type.
----------------------------------------------------------------------------------------------------------


FILE: /Users/interns/Demo-site/drupal-org-issues/web/modules/contrib/comment_notify/comment_notify.migrate.inc
--------------------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
--------------------------------------------------------------------------------------------------------------
 35 | ERROR | Missing parameter type
 37 | ERROR | Missing parameter type
 53 | ERROR | Missing parameter type
 55 | ERROR | Missing parameter type
--------------------------------------------------------------------------------------------------------------


FILE: /Users/interns/Demo-site/drupal-org-issues/web/modules/contrib/comment_notify/tests/src/Functional/CommentNotifyUserPreferencesTest.php
---------------------------------------------------------------------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
---------------------------------------------------------------------------------------------------------------------------------------------
  60 | ERROR | Unnecessarily gendered language in a comment
  75 | ERROR | Unnecessarily gendered language in a comment
  91 | ERROR | Unnecessarily gendered language in a comment
  92 | ERROR | Unnecessarily gendered language in a comment
 106 | ERROR | Unnecessarily gendered language in a comment
---------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/interns/Demo-site/drupal-org-issues/web/modules/contrib/comment_notify/README.md
---------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
---------------------------------------------------------------------------------------------
 14 | WARNING | Line exceeds 80 characters; contains 87 characters
 45 | WARNING | Line exceeds 80 characters; contains 84 characters
 61 | WARNING | Line exceeds 80 characters; contains 83 characters
 88 | WARNING | Line exceeds 80 characters; contains 83 characters
---------------------------------------------------------------------------------------------


FILE: /Users/interns/Demo-site/drupal-org-issues/web/modules/contrib/comment_notify/comment_notify.module
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AND 5 WARNINGS AFFECTING 9 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  22 | WARNING | [ ] Global constants should not be used, move it to a class or interface
  23 | WARNING | [ ] Global constants should not be used, move it to a class or interface
  24 | WARNING | [ ] Global constants should not be used, move it to a class or interface
 176 | ERROR   | [x] Use null coalesce operator instead of ternary operator.
 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()?
 268 | ERROR   | [x] Use null coalesce operator instead of ternary operator.
 285 | ERROR   | [x] Use null coalesce operator instead of ternary operator.
 476 | ERROR   | [ ] Unnecessarily gendered language in a comment
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/interns/Demo-site/drupal-org-issues/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: /Users/interns/Demo-site/drupal-org-issues/web/modules/contrib/comment_notify/src/Form/CommentNotifyUnsubscribe.php
-------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------
 40 | ERROR | Parameter $module_handler is not described in comment
-------------------------------------------------------------------------------------------------------------------------


FILE: /Users/interns/Demo-site/drupal-org-issues/web/modules/contrib/comment_notify/src/Form/CommentNotifySettings.php
----------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------
 65 | ERROR | Parameter $messenger is not described in comment
----------------------------------------------------------------------------------------------------------------------


FILE: /Users/interns/Demo-site/drupal-org-issues/web/modules/contrib/comment_notify/src/UserNotificationSettings.php
--------------------------------------------------------------------------------------------------------------------
FOUND 7 ERRORS AFFECTING 5 LINES
--------------------------------------------------------------------------------------------------------------------
 27 | ERROR | Parameter $configFactory is not described in comment
 32 | ERROR | Missing parameter comment
 32 | ERROR | Doc comment for parameter $configFactory; does not match actual variable name $configFactory
 91 | ERROR | Missing parameter comment
 91 | ERROR | Missing parameter type
 92 | ERROR | Missing parameter type
 95 | ERROR | Description for the @return value is missing
--------------------------------------------------------------------------------------------------------------------

Time: 884ms; Memory: 14MB

Please review.
Thank you.

riddhi.addweb’s picture

Status: Needs review » Needs work

The patch was applied cleanly and after applying the patch the errors are still not resolved.

FILE: /var/www/html/web/drupal10/web/modules/contrib/comment_notify/tests/src/Functional/CommentNotifyUserPreferencesTest.php
-----------------------------------------------------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
-----------------------------------------------------------------------------------------------------------------------------
  60 | ERROR | Unnecessarily gendered language in a comment
  75 | ERROR | Unnecessarily gendered language in a comment
  91 | ERROR | Unnecessarily gendered language in a comment
  92 | ERROR | Unnecessarily gendered language in a comment
 106 | ERROR | Unnecessarily gendered language in a comment
-----------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/drupal10/web/modules/contrib/comment_notify/comment_notify.migrate.inc
----------------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
----------------------------------------------------------------------------------------------
 35 | ERROR | Missing parameter type
 37 | ERROR | Missing parameter type
 53 | ERROR | Missing parameter type
 55 | ERROR | Missing parameter type
----------------------------------------------------------------------------------------------


FILE: /var/www/html/web/drupal10/web/modules/contrib/comment_notify/comment_notify.install
------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------------------
 109 | WARNING | Unused variable $entity_type.
------------------------------------------------------------------------------------------


FILE: /var/www/html/web/drupal10/web/modules/contrib/comment_notify/src/Form/CommentNotifySettings.php
------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------
 65 | ERROR | Parameter $messenger is not described in comment
------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/drupal10/web/modules/contrib/comment_notify/src/Form/CommentNotifyUnsubscribe.php
---------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------
 40 | ERROR | Parameter $module_handler is not described in comment
---------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/drupal10/web/modules/contrib/comment_notify/src/UserNotificationSettings.php
-----------------------------------------------------------------------------------------------------------
FOUND 7 ERRORS AFFECTING 5 LINES
-----------------------------------------------------------------------------------------------------------
 27 | ERROR | Parameter $configFactory is not described in comment
 32 | ERROR | Missing parameter comment
 32 | ERROR | Doc comment for parameter $configFactory; does not match actual variable name $configFactory
 91 | ERROR | Missing parameter comment
 91 | ERROR | Missing parameter type
 92 | ERROR | Missing parameter type
 95 | ERROR | Description for the @return value is missing
-----------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/drupal10/web/modules/contrib/comment_notify/comment_notify.module
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AND 5 WARNINGS AFFECTING 9 LINES
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  22 | WARNING | [ ] Global constants should not be used, move it to a class or interface
  23 | WARNING | [ ] Global constants should not be used, move it to a class or interface
  24 | WARNING | [ ] Global constants should not be used, move it to a class or interface
 176 | ERROR   | [x] Use null coalesce operator instead of ternary operator.
 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()?
 268 | ERROR   | [x] Use null coalesce operator instead of ternary operator.
 285 | ERROR   | [x] Use null coalesce operator instead of ternary operator.
 476 | ERROR   | [ ] Unnecessarily gendered language in a comment
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/drupal10/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: /var/www/html/web/drupal10/web/modules/contrib/comment_notify/README.md
-----------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
-----------------------------------------------------------------------------
 14 | WARNING | Line exceeds 80 characters; contains 87 characters
 45 | WARNING | Line exceeds 80 characters; contains 84 characters
 61 | WARNING | Line exceeds 80 characters; contains 83 characters
 88 | WARNING | Line exceeds 80 characters; contains 83 characters
-----------------------------------------------------------------------------

VinmayiSwamy made their first commit to this issue’s fork.

vinmayiswamy’s picture

Fixed most of the issues reported by phpcs in the same MR.

The below are the remaining issues to be fixed.

$ ./vendor/bin/phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml modules/contrib/comment_notify

FILE: /Users/vinmayi.nidubrolu/projects/drupal103/drupal/modules/contrib/comment_notify/comment_notify.install
--------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------
 109 | WARNING | Unused variable $entity_type.
--------------------------------------------------------------------------------------------------------------


FILE: /Users/vinmayi.nidubrolu/projects/drupal103/drupal/modules/contrib/comment_notify/README.md
-------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
-------------------------------------------------------------------------------------------------
 14 | WARNING | Line exceeds 80 characters; contains 87 characters
 45 | WARNING | Line exceeds 80 characters; contains 84 characters
 61 | WARNING | Line exceeds 80 characters; contains 83 characters
 88 | WARNING | Line exceeds 80 characters; contains 83 characters
-------------------------------------------------------------------------------------------------


FILE: /Users/vinmayi.nidubrolu/projects/drupal103/drupal/modules/contrib/comment_notify/comment_notify.module
-------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
-------------------------------------------------------------------------------------------------------------
 22 | WARNING | Global constants should not be used, move it to a class or interface
 23 | WARNING | Global constants should not be used, move it to a class or interface
 24 | WARNING | Global constants should not be used, move it to a class or interface
-------------------------------------------------------------------------------------------------------------

Time: 459ms; Memory: 16MB

Kindly please check and advise.

Thanks!

gnuget’s picture

Current 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), and tests/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

  1. Run PHPCS with the CI Drupal/DrupalPractice configuration.
  2. Run the complete Comment Notify test suite.
  3. Confirm the PHPCS CI job passes rather than failing as an allowed failure.

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.

gnuget’s picture

Status: Needs work » Needs review

Validation 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.

vinodhini.e’s picture

Hi,

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.

gnuget’s picture

Thank 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!!!

  • gnuget committed 5c9213e8 on 8.x-1.x authored by cleavinjosh
    task: #3445084 Fix the issues reported by phpcs
    
    By: cleavinjosh
    By:...
gnuget’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.