Fix the issues reported by phpcs

Created on 7 July 2023, over 1 year ago

Problem/Motivation

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig /var/www/html/VB/drupal-10.1.0/modules/contrib/popin/

FILE: /var/www/html/VB/drupal-10.1.0/modules/contrib/popin/src/Form/PopinConfigForm.php
---------------------------------------------------------------------------------------------------------------------------------------
FOUND 8 ERRORS AND 4 WARNINGS AFFECTING 11 LINES
---------------------------------------------------------------------------------------------------------------------------------------
9 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Config\ConfigFactoryInterface.
14 | WARNING | [x] Unused use statement
18 | WARNING | [ ] The class short comment should describe what the class does and not simply repeat the class name
21 | ERROR | [ ] Missing short description in doc comment
22 | ERROR | [x] Data types in @var tags need to be fully namespaced
22 | ERROR | [x] Data types in @var tags need to be fully namespaced
26 | ERROR | [ ] Missing short description in doc comment
27 | ERROR | [x] Data types in @var tags need to be fully namespaced
40 | ERROR | [x] Missing function doc comment
160 | WARNING | [ ] Possible useless method overriding detected
170 | ERROR | [x] Expected 1 space after IF keyword; 0 found
171 | WARNING | [ ] File::load calls should be avoided in classes, use dependency injection instead
---------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 7 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------------------------------

FILE: /var/www/html/VB/drupal-10.1.0/modules/contrib/popin/src/Plugin/Block/PopinBlock.php
-----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 5 ERRORS AND 5 WARNINGS AFFECTING 9 LINES
-----------------------------------------------------------------------------------------------------------------------------------------------
11 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Plugin\ContainerFactoryPluginInterface.
12 | WARNING | [x] Unused use statement
13 | WARNING | [x] Unused use statement
14 | WARNING | [x] Unused use statement
24 | ERROR | [x] Expected 1 space before opening brace; found 2
26 | ERROR | [ ] Missing short description in doc comment
31 | ERROR | [ ] Parameter $config_factory is not described in comment
44 | ERROR | [x] Expected 1 blank line after function; 0 found
85 | WARNING | [ ] ImageStyle::load calls should be avoided in classes, use dependency injection instead
85 | WARNING | [ ] File::load calls should be avoided in classes, use dependency injection instead
-----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 6 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------------------------------

FILE: /var/www/html/VB/drupal-10.1.0/modules/contrib/popin/popin.module
-----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------
1 | ERROR | [x] Missing file doc comment
-----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------

FILE: /var/www/html/VB/drupal-10.1.0/modules/contrib/popin/css/popin.css
------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
------------------------------------------------------------------------
78 | ERROR | [x] Expected 1 newline at end of file; 3 found
80 | ERROR | [x] Additional whitespace found at end of file
------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------

FILE: /var/www/html/VB/drupal-10.1.0/modules/contrib/popin/README.md
------------------------------------------------------------------------
FOUND 1 ERROR AND 5 WARNINGS AFFECTING 5 LINES
------------------------------------------------------------------------
15 | WARNING | [ ] Line exceeds 80 characters; contains 92 characters
19 | WARNING | [ ] Line exceeds 80 characters; contains 111 characters
25 | WARNING | [ ] Line exceeds 80 characters; contains 111 characters
27 | WARNING | [ ] Line exceeds 80 characters; contains 94 characters
29 | WARNING | [ ] Line exceeds 80 characters; contains 83 characters
29 | ERROR | [x] Expected 1 newline at end of file; 3 found
------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------

Time: 253ms; Memory: 10MB

Steps to reproduce
Run this command
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig /var/www/html/VB/drupal-10.1.0/modules/contrib/popin/

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

๐Ÿ“Œ Task
Status

Active

Version

2.0

Component

Code

Created by

๐Ÿ‡ฎ๐Ÿ‡ณIndia indrapatil Bangalore

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 @indrapatil
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Shreya_98

    I had fixed all the errors reported by phpcs but some warning are still left . I have attached screenshot of warnings which will be solve by maintainer.

  • Issue was unassigned.
  • Status changed to Needs review about 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น

    I am resetting the Assigned field, since that person did not do anything for this issue after more than 24 hours.

  • Status changed to Needs work about 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น

    @@ -12,20 +12,23 @@ The popin content is composed of :

    Since the file is edited, the spaces before colons must be removed too.

    +The popin will be displayed once per user session. Update the popin
    + configuration form will reset this setting.
     
     ## Installation
     

    There must be two empty lines before a new section.

    -Finally you can configure the popin content and visibility on /admin/content/popin.
    +After that, head to blocks structure configuration to add the ร‚ยซ Popin block ร‚ยป
    + to a region (footer for example).

    In English, text is quoted between double quote characters (").

    +(You can use block visibility settings if you want to display
    + the popin only on the frontpage)

    A period is missing after frontpage.

    +/**
    + * @file
    + * Contains implementation of theme hooks for the Popin module.
    + */
    +

    The usual description is Hook implementations for the [module name] module. where [module name] is replaced by the module name given in the .info.yml file.

       /**
    -   * @var FileUsageInterface
    +   * The file usage service for tracking how files are used within Drupal.
    +   *
    +   * @var \Drupal\file\Entity\FileUsageInterface
        */
       protected $fileUsage;

    The file usage service. is sufficient.

       /**
    -   * @var AccountProxyInterface
    +   * The current user account proxy service.
    +   *
    +   * @var \Drupal\Core\Session\AccountProxyInterface
        */

    The current user. is sufficient.

    +  /**
    +   * Creates a new instance of the Popin configuration form.
    +   *
    +   * @param \Symfony\Component\DependencyInjection\ContainerInterface $container
    +   *   The service container.
    +   *
    +   * @return \Drupal\your_module\PopinConfigForm
    +   *   A new instance of the PopinConfigForm class.
    +   */

    The documentation comment for methods inherited from a parent class or defined in an interface the class implements is {@inheritdoc}.

       /**
    +   * The configuration factory service for managing configuration settings.
    +   *
        * @var \Drupal\Core\Config\ConfigFactoryInterface
        */
       protected $configFactory;

    The configuration factory. is sufficient.

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

    Addressed comment #4. Providing updated patch.

  • Issue was unassigned.
  • Status changed to Needs review about 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India
  • Status changed to Needs work about 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
     ## Installation
     
     You can download this module with composer, and enable it like any other module.
     
    -After that, head to blocks structure configuration to add the ร‚ยซ Popin block ร‚ยป to a region (footer for example).
    -
    -(You can use block visibility settings if you want to display the popin only on the frontpage)
    -
    -Finally you can configure the popin content and visibility on /admin/content/popin.
    +After that, head to blocks structure configuration to add the "Popin block" to a
    +region (footer for example).

    The Installation section should say what the template suggests.

    Install as you would normally install a contributed Drupal module. For further
    information, see
    [Installing Drupal Modules](https://www.drupal.org/docs/extending-drupal/installing-drupal-modules).

    The rest is part of the Configuration section.

    The README.md file is not even complete. It would be better to leave any change for that file to a different issue.

    -Finally you can configure the popin content and visibility on /admin/content/popin.
    +After that, head to blocks structure configuration to add the "Popin block" to a
    +region (footer for example).
    +(You can use block visibility settings if you want to display the popin only on
    +the frontpage).

    SInce that is a parenthetical, the period goes inside the parentheses.

  • First commit to issue fork.
  • First commit to issue fork.
  • Hi,
    Reviewed #10, the patch is not applying and is throwing following error.

    Checking patch README.md...
    error: while searching for:

    All are optionals.

    You can customize the fields order by editing the provided template (block-popin.html.twig).

    You can configure the visibility of the popin by date, or globally.

    The popin will be displayed once per user session. Update the popin configuration form will reset this setting.

    ## Installation

    You can download this module with composer, and enable it like any other module.

    After that, head to blocks structure configuration to add the ยซ Popin block ยป to a region (footer for example).

    (You can use block visibility settings if you want to display the popin only on the frontpage)

    Finally you can configure the popin content and visibility on /admin/content/popin.

    error: patch failed: README.md:12
    error: README.md: patch does not apply
    Checking patch css/popin.css...
    error: while searching for:
    margin-top: 15px;
    text-align: center;
    }

    error: patch failed: css/popin.css:76
    error: css/popin.css: patch does not apply
    Checking patch popin.module...
    error: while searching for:
    <?php

    /**
    * Implements hook_theme().
    */

    error: patch failed: popin.module:1
    error: popin.module: patch does not apply
    Checking patch src/Form/PopinConfigForm.php...
    error: while searching for:
    namespace Drupal\popin\Form;

    use Drupal\Core\Cache\Cache;
    use Drupal\Core\Datetime\DrupalDateTime;
    use Drupal\Core\Form\ConfigFormBase;
    use Drupal\Core\Form\FormStateInterface;
    use Drupal\Core\Config\ConfigFactoryInterface;
    use Drupal\Core\Session\AccountProxyInterface;
    use Drupal\file\Entity\File;
    use Drupal\file\FileUsage\FileUsageInterface;
    use Symfony\Component\DependencyInjection\ContainerInterface;
    use Drupal\Core\Config\ConfigManagerInterface;

    /**
    * Class PopinConfigForm.
    */
    class PopinConfigForm extends ConfigFormBase {

    /**
    * @var FileUsageInterface
    */
    protected $fileUsage;

    /**
    * @var AccountProxyInterface
    */
    protected $currentUser;

    error: patch failed: src/Form/PopinConfigForm.php:3
    error: src/Form/PopinConfigForm.php: patch does not apply
    Checking patch src/Plugin/Block/PopinBlock.php...
    error: while searching for:
    use Drupal\Core\Block\BlockBase;
    use Drupal\Core\Config\ConfigFactoryInterface;
    use Drupal\Core\Datetime\DrupalDateTime;
    use Drupal\file\Entity\File;
    use Drupal\image\Entity\ImageStyle;
    use Symfony\Component\DependencyInjection\ContainerInterface;
    use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
    use Drupal\Core\Session\SessionManagerInterface;
    use Drupal\Core\Session\AccountProxyInterface;
    use Symfony\Component\HttpFoundation\RequestStack;

    /**
    * Provides a 'PopinBlock' block.

    error: patch failed: src/Plugin/Block/PopinBlock.php:5
    error: src/Plugin/Block/PopinBlock.php: patch does not apply

  • Assigned to mohd sahzad
  • Issue was unassigned.
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia zkhan.aamir

    Hi,

    Patch #10 applied successfully.

    Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules/popin (2.0.x)
    $ curl https://www.drupal.org/files/issues/2023-11-29/phpcs-fixes-3373278-10.patch | patch -p1
      % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                     Dload  Upload   Total   Spent    Left  Speed
    100  8006  100  8006    0     0  21012      0 --:--:-- --:--:-- --:--:-- 21124
    patching file README.md
    patching file css/popin.css
    patching file popin.module
    patching file src/Form/PopinConfigForm.php
    patching file src/Plugin/Block/PopinBlock.php
    

    No issues remianing.

    Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules/popin (2.0.x)
    $ cd ../
    
    Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules
    $ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,info,txt,md,css,js,yml popin/
    
    Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules
    $
    
  • Assigned to nitin_lama
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India

    Addressing #8. Providing updated patch.

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

    Please review. Thanks.

  • Issue was unassigned.
  • Status changed to Needs review 9 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia nitin_lama India
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Yashaswi18

    Hello, the patch #19 applies cleanly and there are no errors or warnings remaining when applied without checking out to any branch.

    git apply phpcs-fixes-3373278-17.patch -v
    Checking patch README.md...
    Checking patch css/popin.css...
    Checking patch popin.module...
    Checking patch src/Form/PopinConfigForm.php...
    Checking patch src/Plugin/Block/PopinBlock.php...
    Applied patch README.md cleanly.
    Applied patch css/popin.css cleanly.
    Applied patch popin.module cleanly.
    Applied patch src/Form/PopinConfigForm.php cleanly.
    Applied patch src/Plugin/Block/PopinBlock.php cleanly.
  • Status changed to Needs work 6 months ago
  • Hi,

    Applied patch #17 and found one error

    FILE: ...etInterns/Demo-site/drupal-orgissue/web/modules/contrib/popin/popin.module
    --------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    --------------------------------------------------------------------------------
     1 | ERROR | [x] Missing file doc comment
    --------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    --------------------------------------------------------------------------------
    
    Time: 237ms; Memory: 10MB
    
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia dev16.addweb

    Hello,
    I resolve all listed errors in this patch, please review.

    FILE: popin/css/popin.css
    ---------------------------------------------------------------------------------------------------
    FOUND 2 ERRORS AFFECTING 2 LINES
    ---------------------------------------------------------------------------------------------------
     78 | ERROR | [x] Expected 1 newline at end of file; 3 found
     80 | ERROR | [x] Additional whitespace found at end of file
    ---------------------------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ---------------------------------------------------------------------------------------------------
    
    
    FILE: popin/popin.module
    --------------------------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    --------------------------------------------------------------------------------------------------
     1 | ERROR | [x] Missing file doc comment
    --------------------------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    --------------------------------------------------------------------------------------------------
    
    
    FILE: popin/popin.module
    ----------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------
     3 | ERROR | Missing short description in doc comment
    ----------------------------------------------------------------------
    
  • Status changed to RTBC 6 months ago
  • Hi @silvi.addweb,

    Applied patch #22, confirmed all errors/warnings fixed.

     popin git:(2.0.x) โœ— curl https://www.drupal.org/files/issues/2024-05-30/phpcs-fixes-3373278-22.patch | patch -p1
      % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                     Dload  Upload   Total   Spent    Left  Speed
    100   448  100   448    0     0   1461      0 --:--:-- --:--:-- --:--:--  1503
    patching file css/popin.css
    patching file popin.module
    โžœ  popin git:(2.0.x) โœ— cd ..
    โžœ  contrib git:(main) โœ— phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig popin
    โžœ  contrib git:(main) โœ—

    Will move this to RTBC.

    Thanks,
    Jake

  • Pipeline finished with Success
    6 months ago
    Total: 155s
    #185783
  • Status changed to Needs work 6 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น

    Since GitLab CI has been enabled for this project, and since there have been commits in the project that fixed some of the reported PHP_Codesniffer warnings/errors, a new MR must be provided. In fact, a single error still needs to be fixed.

  • Status changed to Needs review 6 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
  • Pipeline finished with Success
    6 months ago
    Total: 234s
    #185809
  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine Andrii Momotov

    Thank you all!

  • Status changed to Fixed 5 months ago
  • Status changed to Fixed 5 months ago
Production build 0.71.5 2024