Fix the issues reported by phpcs

Created on 24 January 2023, over 1 year ago
Updated 24 May 2024, about 1 month ago

Problem/Motivation

Getting following warnings.

FILE: /var/www/html/modules/contrib/browsersync/browsersync.module
-------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-------------------------------------------------------------------------------------
15 | WARNING | Global constants should not be used, move it to a class or interface
22 | WARNING | Global constants should not be used, move it to a class or interface
-------------------------------------------------------------------------------------

Time: 1.37 secs; Memory: 6MB

Steps to reproduce

Run following command

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

Proposed resolution

Above warnings need to be fixed.

๐Ÿ“Œ Task
Status

Needs work

Version

3.0

Component

Code

Created by

๐Ÿ‡ฎ๐Ÿ‡ณIndia samit.310@gmail.com

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 @samit.310@gmail.com
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia samit.310@gmail.com

    Above warnings are fixed.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sahilgidwani Jaipur

    I will review patch and will update the status once done.
    Also, please try to follow the MR workflow for new issues.

  • Issue was unassigned.
  • Status changed to RTBC over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sahilgidwani Jaipur

    I have checked and reviewed MR and it works perfectly for me.
    Moving it to RTBC.

  • Assigned to Charchil Khandelwal
  • Merge request !23.0.x โ†’ (Open) created by Charchil Khandelwal
  • Issue was unassigned.
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Charchil Khandelwal

    Created MR for this issue.
    Please review.

  • Status changed to Needs work over 1 year ago
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia elimw

    Rather than calling the class "BrowsersyncConstants", please rename it to "BrowsersyncHelper" and remove the "final" keyword from the class declaration.

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

    i'll work on this ..

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia elimw
  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia samit.310@gmail.com

    Hi @elimw,

    changes has been made. Please review.

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

    +/**
    + * Block Token Constants.
    + */

    That does not describe the class. Constants should not be capitalized, since it is not at the begin of a sentence.
    I am not sure I would even call those constants Block Token constants, since they are used in the definition of the browsersync_snippet theme function.

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia elimw

    Please make all changes to "Issue fork browsersync-3335994" rather than the old patch file method.

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

  • Pipeline finished with Failed
    3 months ago
    Total: 165s
    #119000
  • Pipeline finished with Failed
    3 months ago
    Total: 169s
    #119003
  • Status changed to Needs review 3 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
  • Pipeline finished with Failed
    3 months ago
    Total: 222s
    #119040
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Preethy_ray

    pray_12 โ†’ made their first commit to this issueโ€™s fork.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Preethy_ray

    Fixed Line indented incorrectly for browsersync.module.

  • Pipeline finished with Failed
    3 months ago
    Total: 138s
    #119169
  • ๐Ÿ‡ต๐Ÿ‡ญPhilippines cleavinjosh

    Hi @pray_12,

    I applied MR!2 and confirmed that the issues found on the browsersync.module file by the phpcs are fixed.

    However, I am still encountering some issues when I run phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml

    โžœ  browsersync git:(main) โœ— curl https://git.drupalcode.org/project/browsersync/-/merge_requests/2.diff | patch -p1
      % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                     Dload  Upload   Total   Spent    Left  Speed
    100  8165    0  8165    0     0  18933      0 --:--:-- --:--:-- --:--:-- 18944
    patching file .gitlab-ci.yml
    patching file browsersync.module
    patching file src/BrowserSyncConstants.php
    โžœ  browsersync git:(main) โœ— ..
    โžœ  contrib git:(main) โœ— phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml browsersync
    
    FILE: /Users/interns/Demo-site/drupal-orgissue/web/modules/contrib/browsersync/browsersync.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
    -------------------------------------------------------------------------------------------------------------
    
    Time: 190ms; Memory: 10MB
    
    โžœ  contrib git:(main) โœ—

    I will retain the status to Needs review so that others can confirm as well.

    Thank you.

  • ๐Ÿ‡ต๐Ÿ‡ญPhilippines cleavinjosh
  • Status changed to Needs work about 1 month ago
  • ๐Ÿ‡ต๐Ÿ‡ญPhilippines cleavinjosh
Production build 0.69.0 2024