Fix the issues reported by phpcs

Created on 25 April 2023, over 1 year ago
Updated 28 June 2024, 5 months ago

Problem/Motivation

FILE: /home/system/Documents/contribution/password_notification/password_notification.module
--------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------------------
 118 | WARNING | Unused variable $save_data.
--------------------------------------------------------------------------------------------

Time: 47ms; Memory: 8MB

Steps to reproduce

Execute the command: phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,js,info,txt,md,yml,twig password_notification/

Proposed resolution

Fix all the issues reported for Drupal and DrupalPractice coding standard

Remaining tasks

Patch review and fix the remaining issues.
FILE: ...al9/web/modules/contrib/password_notification/password_notification.module
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
118 | WARNING | Unused variable $save_data.
--------------------------------------------------------------------------------

FILE: ...les/contrib/password_notification/includes/password_notification.admin.inc
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 5 WARNINGS AFFECTING 5 LINES
--------------------------------------------------------------------------------
37 | WARNING | All variables defined by your module must be prefixed with your
| | module's name to avoid name collisions with others. Expected
| | start with "password_notification" but found "mail_subject"
44 | WARNING | All variables defined by your module must be prefixed with your
| | module's name to avoid name collisions with others. Expected
| | start with "password_notification" but found "mail_body"
50 | WARNING | All variables defined by your module must be prefixed with your
| | module's name to avoid name collisions with others. Expected
| | start with "password_notification" but found "mail_bcc"
56 | WARNING | All variables defined by your module must be prefixed with your
| | module's name to avoid name collisions with others. Expected
| | start with "password_notification" but found "mail_cc"
68 | WARNING | All variables defined by your module must be prefixed with your
| | module's name to avoid name collisions with others. Expected
| | start with "password_notification" but found
| | "delete_records_limit"
--------------------------------------------------------------------------------

Time: 539ms; Memory: 10MB

๐Ÿ“Œ Task
Status

RTBC

Version

3.0

Component

Code

Created by

๐Ÿ‡ฎ๐Ÿ‡ณIndia urvashi_vora Madhya Pradesh, India

Live updates comments and jobs are added and updated live.
  • Coding standards

    It involves compliance with, or the content of coding standards. Requires broad community agreement.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @urvashi_vora
  • Status changed to Needs work 10 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Ashutosh Ahirwal India

    Hi Provided patch get applied cleanly but its still showing some error.
    Moving to Need work.

    Used command with arguments
    ./vendor/bin/phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,info,txt,md,css,js,yml web/modules/custom/password_notification

    FILE: /Users/ashutoshahirwal/Sites/lando/contribution/web/modules/custom/password_notification/includes/password_notification.admin.inc
    ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 5 WARNINGS AFFECTING 5 LINES
    ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
    37 | WARNING | All variables defined by your module must be prefixed with your module's name to avoid name collisions with others. Expected start with "password_notification" but found "mail_subject"
    44 | WARNING | All variables defined by your module must be prefixed with your module's name to avoid name collisions with others. Expected start with "password_notification" but found "mail_body"
    50 | WARNING | All variables defined by your module must be prefixed with your module's name to avoid name collisions with others. Expected start with "password_notification" but found "mail_bcc"
    56 | WARNING | All variables defined by your module must be prefixed with your module's name to avoid name collisions with others. Expected start with "password_notification" but found "mail_cc"
    68 | WARNING | All variables defined by your module must be prefixed with your module's name to avoid name collisions with others. Expected start with "password_notification" but found
    | | "delete_records_limit"
    ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

    FILE: /Users/ashutoshahirwal/Sites/lando/contribution/web/modules/custom/password_notification/password_notification.module
    ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
    FOUND 2 ERRORS AND 1 WARNING AFFECTING 3 LINES
    ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
    118 | WARNING | Unused variable $save_data.
    152 | ERROR | All functions defined in a module file must be prefixed with the module's name, found "password_changed_history_delete_form" but expected
    | | "password_notification_password_changed_history_delete_form"
    184 | ERROR | All functions defined in a module file must be prefixed with the module's name, found "password_changed_history_delete_form_submit" but expected
    | | "password_notification_password_changed_history_delete_form_submit"
    ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

    Time: 307ms; Memory: 12MB

  • Assigned to nitin_lama
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India

    Addressed #2. Fixed most of the issues. Only one issue remaining.

    FILE: /home/system/Documents/contribution/password_notification/password_notification.module
    --------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    --------------------------------------------------------------------------------------------
     118 | WARNING | Unused variable $save_data.
    --------------------------------------------------------------------------------------------
    
    Time: 47ms; Memory: 8MB
    

    Providing updated patch.

  • Issue was unassigned.
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India
  • First commit to issue fork.
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia pray_12

    pray_12 โ†’ changed the visibility of the branch phpcs-fix3356277- to hidden.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia zkhan.aamir

    Updated issue summary

  • Assigned to Anita verma
  • Issue was unassigned.
  • Status changed to Needs review 9 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Anita verma

    Hi

    I applied #4 patch coding-standard-fixes_4.patch it's fixed all the errors, except one warning, we can ignore this warning.

    Thanks

  • Status changed to Needs work 6 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia dev16.addweb
    FILE: /home/addweb/Drupal8-vagrant/web/web/drupal_10_2_6/web/modules/contrib/password_notification/src/Form/PasswordChangedHistoryForm.php
    ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
    FOUND 14 ERRORS AND 6 WARNINGS AFFECTING 16 LINES
    ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
      42 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
      50 | ERROR   | The $_GET super global must not be accessed directly; inject the request_stack service and use $stack->getCurrentRequest()->query->get('username') instead
      51 | ERROR   | The $_GET super global must not be accessed directly; inject the request_stack service and use $stack->getCurrentRequest()->query->get('username') instead
      52 | ERROR   | The $_GET super global must not be accessed directly; inject the request_stack service and use $stack->getCurrentRequest()->query->get('username') instead
      55 | ERROR   | The $_GET super global must not be accessed directly; inject the request_stack service and use $stack->getCurrentRequest()->query->get('email') instead
      56 | ERROR   | The $_GET super global must not be accessed directly; inject the request_stack service and use $stack->getCurrentRequest()->query->get('email') instead
      57 | ERROR   | The $_GET super global must not be accessed directly; inject the request_stack service and use $stack->getCurrentRequest()->query->get('email') instead
      59 | ERROR   | The $_GET super global must not be accessed directly; inject the request_stack service and use $stack->getCurrentRequest()->query->get('start_date') instead
      59 | ERROR   | The $_GET super global must not be accessed directly; inject the request_stack service and use $stack->getCurrentRequest()->query->get('end_date') instead
      60 | ERROR   | The $_GET super global must not be accessed directly; inject the request_stack service and use $stack->getCurrentRequest()->query->get('start_date') instead
      62 | ERROR   | The $_GET super global must not be accessed directly; inject the request_stack service and use $stack->getCurrentRequest()->query->get('end_date') instead
      93 | ERROR   | The $_GET super global must not be accessed directly; inject the request_stack service and use $stack->getCurrentRequest()->query->get('username') instead
      93 | ERROR   | The $_GET super global must not be accessed directly; inject the request_stack service and use $stack->getCurrentRequest()->query->get('email') instead
      93 | ERROR   | The $_GET super global must not be accessed directly; inject the request_stack service and use $stack->getCurrentRequest()->query->get('start_date') instead
      93 | ERROR   | The $_GET super global must not be accessed directly; inject the request_stack service and use $stack->getCurrentRequest()->query->get('end_date') instead
     161 | WARNING | Form error messages are user facing text and must run through t() for translation
     202 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
     220 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
     243 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
     246 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
    ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
    
    
    FILE: /home/addweb/Drupal8-vagrant/web/web/drupal_10_2_6/web/modules/contrib/password_notification/src/Form/PasswordChangedHistoryDeleteForm.php
    ------------------------------------------------------------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 10 WARNINGS AFFECTING 10 LINES
    ------------------------------------------------------------------------------------------------------------------------------------------------
     28 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
     44 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     57 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
     60 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
     72 | 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
     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
     86 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
     96 | WARNING | Messages are user facing text and must run through t() for translation
    ------------------------------------------------------------------------------------------------------------------------------------------------
    
    
    FILE: /home/addweb/Drupal8-vagrant/web/web/drupal_10_2_6/web/modules/contrib/password_notification/password_notification.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
    ---------------------------------------------------------------------------------------------------------------------------------

    Still, I'm getting a few errors reported but phpcs.

  • Arijit Acharya โ†’ made their first commit to this issueโ€™s fork.

  • Raised an MR for the issue. Please review and merge.

  • Status changed to Needs review 6 months ago
  • Status changed to RTBC 5 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia dev16.addweb

    Hello,

    I reviewed MR!3 and confirmed it fixes all the errors and warnings reported by phpcs.

    I added screenshots for reference.
    Thank You.

Production build 0.71.5 2024