Fix the issues reported by phpcs

Created on 11 May 2023, about 1 year ago
Updated 7 June 2024, 18 days ago
๐Ÿ“Œ Task
Status

Needs review

Component

Code

Created by

๐Ÿ‡ฎ๐Ÿ‡ณIndia himanshu_jhaloya Mandsaur

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 @himanshu_jhaloya
  • Issue was unassigned.
  • Status changed to Needs review about 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia himanshu_jhaloya Mandsaur
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia arpitk

    Hi @himanshu_jhaloya I reviews the MR and it fixes all reported issues no more warning or error to be fixed. Here attaching screenshots for before and after fix.

    After fix running phpcs --standard=Drupal,DrupalPractice returns clean.

    Thanks!

  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น

    The report given in the issue summary is not correct: It reports warnings for the walkme.info.yml file, but that file does not have lines that needs to be removed.

    name: WalkMe Integration
    type: module
    description: 'Very basic module for implementing WalkMeโ„ข within Drupal.'
    core_version_requirement: ^8 || ^9 || ^10
    package: WalkMe
    configure: admin/config/system/walkme
    
  • Status changed to Needs work about 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น

    If I run the command shown in the issue summary, I get a longer report. I am going to update the issue summary in minutes.

  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
  • Status changed to Needs review about 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
  • Status changed to Needs work about 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
    -Before installing this module go to walkme.com, generate the script and publish script.
    -After module is installed navigate to admin/config/system/walkme and place walkme script in Script field. 
    -
    +Before installing this module go to walkme.com,
    +generate the script and publish script.
    +After module is installed navigate to 
    +admin/config/system/walkme and place walkme script in Script field.

    Avoiding that lines exceed 80 characters does not mean shorting them to 40 characters or less. There is no rule that says lines must be split after punctuation marks.

    It does not seem the MR is removing one of the newlines found at the end of the README.md file.

     /**
    - * Class WalkmeForm.
    + * The WalkmeForm class extends the ConfigFormBase class and represents a form.

    That description does not describe what that class does, except saying it represent a form, which is implicit in the class name.
    A class description must not say which is the parent class nor repeat the class name.

  • Assigned to imustakim
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia imustakim Ahmedabad
  • @imustakim opened merge request.
  • Issue was unassigned.
  • Status changed to Needs review about 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia imustakim Ahmedabad
  • Status changed to RTBC 19 days ago
  • Hi @imustakim,

    Applied your latest changes on MR !6 against 8.x-1.x version, it applied not-so successfully but it fixed all errors. Please see below:

    walkme git:(8.x-1.x) โœ— curl https://git.drupalcode.org/project/walkme/-/merge_requests/6.diff | patch -p1
      % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                     Dload  Upload   Total   Spent    Left  Speed
    100  3278    0  3278    0     0   7283      0 --:--:-- --:--:-- --:--:--  7484
    patching file README.md
    Hunk #1 FAILED at 1.
    1 out of 1 hunk FAILED -- saving rejects to file README.md.rej
    patching file src/Form/WalkmeForm.php
    patching file walkme.routing.yml
    โžœ  walkme git:(8.x-1.x) โœ— cd ..
    โžœ  contrib git:(main) โœ— phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig walkme
    โžœ  contrib git:(main) โœ—

    Will move this to RTBC

    Thanks,
    Jake

  • Pipeline finished with Success
    18 days ago
    Total: 136s
    #193778
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
  • Status changed to Needs review 18 days ago
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly apaderno Brescia, ๐Ÿ‡ฎ๐Ÿ‡น
  • Pipeline finished with Success
    18 days ago
    Total: 140s
    #193782
Production build 0.69.0 2024