Fix phpcs --standard=Drupal and phpcs --standard=DrupalPractice for the Project Browser module [#3238572]

Problem/Motivation

It is hard to follow the code when it's not following the Drupal standard and Drupal Practice.
And that will be harder to commit or fix with MR or a patch.
Our eyes are used to read code with Drupal standards.

Standard check and linting tools are outdated.
And a new linting tool with yarn lint:yaml was added to Drupal core
#2591827: Add YAML linting to core coding standards checks

Very important to make sure that all .yml files are in the right format.
eslint-plugin-yml a good tool to lint with
Example: using not standard format
or check on any duplicate yaml elements

Proposed resolution

Add the yarn lint:yaml
and update all outdated check configs and the package.json file

Compiling Provided Component Styles

In case of wanting to add a new feature or fix a bug in styling.

Get Needed Yarn Development Dependencies
yarn install

After any change of code please check the standards before uploading a patch or creating a MR.

Check Standards/Practice Coding And Linting

Check Drupal Standard And Practice Coding
Check Drupal standard and practice coding.
yarn phpcs

PHP Code Beautifier and Fixer
Fix many errors and warnings automatically.
yarn phpcbf
Linting YAML files
Check all .yml files with Drupal standard yaml format.
yarn lint:yaml
Linting JavaScript files
Check all JavaScript .js, .json files with Drupal standard scripting format.
yarn lint:js
Linting Styling
Check all styling .css files or css in twig or apps with Drupal standard styling format and order.
yarn lint:css

  "scripts": {
    "phpcs": "phpcs --standard=./.phpcs.xml .",
    "phpcbf": "phpcbf --standard=./.phpcs.xml .",
    "lint:yaml": "node ./node_modules/eslint/bin/eslint.js --config=.eslintrc.json --ext .yml .",
    "lint:js": "node ./node_modules/eslint/bin/eslint.js --config=.eslintrc.json .",
    "lint:css": "npx stylelint --config=.stylelintrc.json ."
  },

Remaining tasks

✅ File an issue about this project
✅ Have PSR, PHPCS, lint:js, lint:css, linkt:yaml
❌ Fix all reported standard points to set the floor clean for more collaborative work
❌ Testing to ensure no regression
❌ Automated unit/functional testing coverage
❌ Developer Documentation support on feature change/addition
❌ User Guide Documentation support on feature change/addition
❌ Code review from core team member
❌ Full testing and approval
❌ Credit contributors
❌ Review
❌ Release

User interface changes

None

API changes

None

Data model changes

None

Comment	File	Size	Author
#35	Captura de tela de 2022-07-08 09-38-42.png	135.68 KB	elber
#14	3238572-14.patch	33.99 KB	rajab natshah
#7	Browse-Drupal-org-projects---dev-drupal9-c1-project_browser---25-09-2021--10-15.txt	271.64 KB	rajab natshah
#6	3238572-6.patch	488.11 KB	rajab natshah

Issue fork project_browser-3238572

Show commands

Start within a Git clone of the project using the version control instructions.

Add & fetch this issue fork’s repository

Or, if you do not have SSH keys set up on git.drupalcode.org:

Add & fetch this issue fork’s repository

jul_2022_phpcs changes, plain diff MR !193
Check out this branch for the first time

Check out existing branch, if you already have it locally
3238572-fix-phpcs---standarddrupal changes, plain diff MR !20
Check out this branch for the first time

Check out existing branch, if you already have it locally

About issue forks

Comments

Comment #1

22 September 2021 at 14:29

RajabNatshah created an issue. See original summary.

Comment #2

rajab natshah

he/him

commented 22 September 2021 at 14:33

Title:

Add lint:yaml and update the package.json for the Project Browser module with the latest Drupal 9.3.x dev tools

» Add lint:yaml, lint:js, lint:css to package.json for the Project Browser module with the latest Drupal 9.3.x dev tools

Comment #3

rajab natshah

he/him

commented 22 September 2021 at 15:27

In case we are going to use PostCSS not SASS then the command for the needed dev packages needs to be changed.

    "build:css": "node ./scripts/css/postcss-build.js",
    "watch:css": "node ./scripts/css/postcss-watch.js",

Maybe this project does not have .css files at this time.
Tested only using Drupal core standard set of classes. it is the better option in all cases

Drupal core is using number of yarn commands at
https://git.drupalcode.org/project/drupal/-/blob/9.3.x/core/package.json

  "engines": {
    "yarn": ">= 1.6",
    "node": ">= 12.0"
  },
  "scripts": {
    "build": "yarn build:css & yarn build:js & yarn build:jqueryui",
    "watch": "yarn watch:css & yarn watch:js",
    "build:css": "node ./scripts/css/postcss-build.js",
    "watch:css": "node ./scripts/css/postcss-watch.js",
    "build:js": "cross-env BABEL_ENV=legacy node ./scripts/js/babel-es6-build.js",
    "build:js-dev": "cross-env NODE_ENV=development BABEL_ENV=legacy node ./scripts/js/babel-es6-build.js",
    "build:jqueryui": "cross-env NODE_ENV=development node ./scripts/js/jqueryui-build.js",
    "watch:js": "cross-env BABEL_ENV=legacy node ./scripts/js/babel-es6-watch.js",
    "watch:js-dev": "cross-env NODE_ENV=development BABEL_ENV=legacy node ./scripts/js/babel-es6-watch.js",
    "lint:core-js": "node ./node_modules/eslint/bin/eslint.js .",
    "lint:core-js-passing": "node ./node_modules/eslint/bin/eslint.js --quiet --config=.eslintrc.passing.json .",
    "lint:core-js-stats": "node ./node_modules/eslint/bin/eslint.js --format=./scripts/js/eslint-stats-by-type.js .",
    "lint:css": "stylelint \"**/*.css\"",
    "lint:css-checkstyle": "stylelint \"**/*.css\" --custom-formatter ./node_modules/stylelint-checkstyle-formatter/index.js",
    "lint:yaml": "node ./node_modules/eslint/bin/eslint.js --ext .yml .",
    "test:nightwatch": "cross-env BABEL_ENV=development node -r dotenv-safe/config -r @babel/register ./node_modules/.bin/nightwatch --config ./tests/Drupal/Nightwatch/nightwatch.conf.js",
    "prettier": "prettier --write \"./**/*.es6.js\" \"./tests/Drupal/Nightwatch/**/*.js\"",
    "spellcheck": "cspell",
    "spellcheck:make-drupal-dict": "rm -f misc/cspell/dictionary.txt && yarn -s spellcheck:core --unique --wordsOnly | tr '[:upper:]' '[:lower:]' | tr -d \\\\\\\\ | LC_ALL=C sort -u -o misc/cspell/dictionary.txt",
    "spellcheck:core": "cspell \"**/*\" \".*\" \"../composer/**/*\" \"../composer.json\"",
    "vendor-update": "node ./scripts/js/assets.js"
  },

Comment #4

rajab natshah

he/him

commented 22 September 2021 at 15:44

Issue summary:

View changes

Comment #5

rajab natshah

he/him

commented 22 September 2021 at 15:45

Issue summary:

View changes

Comment #6

rajab natshah

he/him

commented 25 September 2021 at 07:04

Status	File	Size
new	3238572-6.patch	488.11 KB

Comment #7

rajab natshah

he/him

commented 25 September 2021 at 07:31

Assigned:	rajab natshah	» Unassigned
Issue summary:	View changes
Status:	Active	» Needs review

Status	File	Size
new	Browse-Drupal-org-projects---dev-drupal9-c1-project_browser---25-09-2021--10-15.txt	271.64 KB

Attached the output check report for

phpcs --standard=Drupal

rajab@vardot-dev:/var/www/html/products/project_browser$ phpcs --standard=Drupal --extensions=php,inc,install,test,profile,theme,css,info,txt,md,yml /var/www/html/products/project_browser/ --ignore="/var/www/html/products/project_browser/node_modules/ , /var/www/html/products/project_browser/sveltejs/node_modules/"

phpcs --standard=DrupalPractice

rajab@vardot-dev:/var/www/html/products/project_browser$ phpcs --standard=DrupalPractice --extensions=php,inc,install,test,profile,theme,css,info,txt,md,yml /var/www/html/products/project_browser/ --ignore="/var/www/html/products/project_browser/node_modules/ , /var/www/html/products/project_browser/sveltejs/node_modules/"

yarn phpcs

rajab@vardot-dev:/var/www/html/products/project_browser$ yarn phpcs
yarn run v1.22.10
$ phpcs --standard=./.phpcs.xml .

yarn phpcbf

rajab@vardot-dev:/var/www/html/products/project_browser$ yarn phpcbf

yarn lint:yaml

rajab@vardot-dev:/var/www/html/products/project_browser$ yarn lint:yaml
yarn run v1.22.10
$ node ./node_modules/eslint/bin/eslint.js --config=.eslintrc.json --ext .yml .

yarn lint:js

rajab@vardot-dev:/var/www/html/products/project_browser$ yarn lint:js
yarn run v1.22.10
$ node ./node_modules/eslint/bin/eslint.js --config=.eslintrc.json --ext .js .

yarn lint:css

rajab@vardot-dev:/var/www/html/products/project_browser$ yarn lint:css
yarn run v1.22.10
$ npx stylelint --config=.stylelintrc.json .

Not sure if we should fix all reported standard issues in this issue.

Comment #8

rajab natshah

he/him

commented 25 September 2021 at 07:36

Issue summary:

View changes

Comment #9

rajab natshah

he/him

commented 4 October 2021 at 12:21

Issue summary:

View changes

Comment #10

gaurav.kapoor commented 12 October 2021 at 09:48

+1 to the fact that coding standards aren't as per Drupal in most of the files. The text file attached in #7 highlights all the issues as well. But do we need all the files provided in #6? I am not sure about it. Can't we just fix CS issues and push them and take care of them when new patches/MR are provided.

Comment #11

rajab natshah

he/him

commented 12 October 2021 at 10:29

Title:

Add lint:yaml, lint:js, lint:css to package.json for the Project Browser module with the latest Drupal 9.3.x dev tools

» Add PHPCS, lint:yaml, lint:js, lint:css to package.json for the Project Browser module with the latest Drupal 9.3.x dev tools

Agrees with Gaurav.
Updated the title to reflect that.

Comment #12

rajab natshah

he/him

commented 17 November 2021 at 09:12

Title:	Add PHPCS, lint:yaml, lint:js, lint:css to package.json for the Project Browser module with the latest Drupal 9.3.x dev tools	» Fix phpcs --standard=Drupal and phpcs --standard=DrupalPractice for the Project Browser module
Assigned:	Unassigned	» rajab natshah
Status:	Needs review	» Active

phpcs --standard=Drupal

phpcs --standard=Drupal --extensions=php,inc,install,test,profile,theme,css,info,txt,md,yml /var/www/html/products/project_browser/ --ignore="/var/www/html/products/project_browser/node_modules/ , /var/www/html/products/project_browser/sveltejs/node_modules/"

FILE: /var/www/html/products/project_browser/README.md
----------------------------------------------------------------------
FOUND 0 ERRORS AND 16 WARNINGS AFFECTING 16 LINES
----------------------------------------------------------------------
   9 | WARNING | Line exceeds 80 characters; contains 166 characters
  17 | WARNING | Line exceeds 80 characters; contains 130 characters
  58 | WARNING | Line exceeds 80 characters; contains 90 characters
  59 | WARNING | Line exceeds 80 characters; contains 125 characters
  60 | WARNING | Line exceeds 80 characters; contains 118 characters
  62 | WARNING | Line exceeds 80 characters; contains 118 characters
  63 | WARNING | Line exceeds 80 characters; contains 116 characters
  64 | WARNING | Line exceeds 80 characters; contains 130 characters
  87 | WARNING | Line exceeds 80 characters; contains 117 characters
  88 | WARNING | Line exceeds 80 characters; contains 116 characters
  89 | WARNING | Line exceeds 80 characters; contains 117 characters
  91 | WARNING | Line exceeds 80 characters; contains 119 characters
  92 | WARNING | Line exceeds 80 characters; contains 115 characters
 100 | WARNING | Line exceeds 80 characters; contains 84 characters
 101 | WARNING | Line exceeds 80 characters; contains 82 characters
 104 | WARNING | Line exceeds 80 characters; contains 81 characters
----------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/src/ProjectBrowserEndpointInterface.php
------------------------------------------------------------------------------------
FOUND 3 ERRORS AND 3 WARNINGS AFFECTING 6 LINES
------------------------------------------------------------------------------------
  8 | ERROR   | [ ] Missing short description in doc comment
  9 | WARNING | [x] '@todo.' should match the format '@todo Fix problem X here.'
 13 | ERROR   | [ ] Missing short description in doc comment
 14 | WARNING | [x] '@todo.' should match the format '@todo Fix problem X here.'
 18 | ERROR   | [ ] Missing short description in doc comment
 19 | WARNING | [x] '@todo.' should match the format '@todo Fix problem X here.'
------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/src/DrupalOrg/DrupalOrgProject.php
-----------------------------------------------------------------------------------------------------------------
FOUND 105 ERRORS AFFECTING 65 LINES
-----------------------------------------------------------------------------------------------------------------
  14 | ERROR | Missing @var tag in member variable comment
  16 | ERROR | Missing member variable doc comment
  17 | ERROR | Missing member variable doc comment
  18 | ERROR | Missing member variable doc comment
  19 | ERROR | Class property $changed_ago should use lowerCamel naming without underscores
  19 | ERROR | Missing member variable doc comment
  20 | ERROR | Class property $field_project_images should use lowerCamel naming without underscores
  20 | ERROR | Missing member variable doc comment
  21 | ERROR | Missing member variable doc comment
  22 | ERROR | Missing member variable doc comment
  23 | ERROR | Missing member variable doc comment
  24 | ERROR | Missing member variable doc comment
  25 | ERROR | Missing member variable doc comment
  26 | ERROR | Class property $project_usage should use lowerCamel naming without underscores
  26 | ERROR | Missing member variable doc comment
  27 | ERROR | Class property $project_usage_total should use lowerCamel naming without underscores
  27 | ERROR | Missing member variable doc comment
  28 | ERROR | Class property $field_security_advisory_coverage should use lowerCamel naming without underscores
  28 | ERROR | Missing member variable doc comment
  29 | ERROR | Class property $field_project_machine_name should use lowerCamel naming without underscores
  29 | ERROR | Missing member variable doc comment
  30 | ERROR | Class property $field_supporting_organizations should use lowerCamel naming without underscores
  30 | ERROR | Missing member variable doc comment
  31 | ERROR | Class property $flag_project_star_user_count should use lowerCamel naming without underscores
  31 | ERROR | Missing member variable doc comment
  35 | ERROR | Missing @var tag in member variable comment
  36 | ERROR | Class property $taxonomy_vocabulary_3 should use lowerCamel naming without underscores
  40 | ERROR | Missing @var tag in member variable comment
  41 | ERROR | Class property $taxonomy_vocabulary_44 should use lowerCamel naming without underscores
  45 | ERROR | Missing @var tag in member variable comment
  46 | ERROR | Class property $taxonomy_vocabulary_46 should use lowerCamel naming without underscores
  50 | ERROR | Missing @var tag in member variable comment
  52 | ERROR | Missing member variable doc comment
  53 | ERROR | Class property $edit_url should use lowerCamel naming without underscores
  53 | ERROR | Missing member variable doc comment
  54 | ERROR | Missing member variable doc comment
  55 | ERROR | Class property $is_new should use lowerCamel naming without underscores
  55 | ERROR | Missing member variable doc comment
  56 | ERROR | Missing member variable doc comment
  57 | ERROR | Class property $flag_project_star_user should use lowerCamel naming without underscores
  57 | ERROR | Missing member variable doc comment
  58 | ERROR | Class property $field_issue_summary_template should use lowerCamel naming without underscores
  58 | ERROR | Missing member variable doc comment
  59 | ERROR | Class property $field_replaced_by should use lowerCamel naming without underscores
  59 | ERROR | Missing member variable doc comment
  60 | ERROR | Class property $field_next_major_version_info should use lowerCamel naming without underscores
  60 | ERROR | Missing member variable doc comment
  61 | ERROR | Class property $field_project_ecosystem should use lowerCamel naming without underscores
  61 | ERROR | Missing member variable doc comment
  62 | ERROR | Class property $field_project_issue_version_opts should use lowerCamel naming without underscores
  62 | ERROR | Missing member variable doc comment
  63 | ERROR | Class property $field_project_components should use lowerCamel naming without underscores
  63 | ERROR | Missing member variable doc comment
  64 | ERROR | Class property $field_project_docs should use lowerCamel naming without underscores
  64 | ERROR | Missing member variable doc comment
  65 | ERROR | Class property $field_project_license should use lowerCamel naming without underscores
  65 | ERROR | Missing member variable doc comment
  66 | ERROR | Class property $field_project_screenshots should use lowerCamel naming without underscores
  66 | ERROR | Missing member variable doc comment
  67 | ERROR | Class property $field_project_documentation should use lowerCamel naming without underscores
  67 | ERROR | Missing member variable doc comment
  68 | ERROR | Class property $field_project_demo should use lowerCamel naming without underscores
  68 | ERROR | Missing member variable doc comment
  69 | ERROR | Missing member variable doc comment
  70 | ERROR | Class property $field_project_changelog should use lowerCamel naming without underscores
  70 | ERROR | Missing member variable doc comment
  71 | ERROR | Class property $field_project_homepage should use lowerCamel naming without underscores
  71 | ERROR | Missing member variable doc comment
  72 | ERROR | Class property $field_release_version_format should use lowerCamel naming without underscores
  72 | ERROR | Missing member variable doc comment
  73 | ERROR | Class property $field_project_has_releases should use lowerCamel naming without underscores
  73 | ERROR | Missing member variable doc comment
  74 | ERROR | Class property $field_project_issue_guidelines should use lowerCamel naming without underscores
  74 | ERROR | Missing member variable doc comment
  75 | ERROR | Class property $field_project_default_component should use lowerCamel naming without underscores
  75 | ERROR | Missing member variable doc comment
  76 | ERROR | Class property $field_project_has_issue_queue should use lowerCamel naming without underscores
  76 | ERROR | Missing member variable doc comment
  77 | ERROR | Class property $field_project_type should use lowerCamel naming without underscores
  77 | ERROR | Missing member variable doc comment
  78 | ERROR | Class property $last_comment_timestamp should use lowerCamel naming without underscores
  78 | ERROR | Missing member variable doc comment
  79 | ERROR | Class property $has_new_content should use lowerCamel naming without underscores
  79 | ERROR | Missing member variable doc comment
  80 | ERROR | Class property $flag_tracker_follower_count should use lowerCamel naming without underscores
  80 | ERROR | Missing member variable doc comment
  81 | ERROR | Class property $feed_nid should use lowerCamel naming without underscores
  81 | ERROR | Missing member variable doc comment
  82 | ERROR | Class property $feeds_item_url should use lowerCamel naming without underscores
  82 | ERROR | Missing member variable doc comment
  83 | ERROR | Class property $feeds_item_guid should use lowerCamel naming without underscores
  83 | ERROR | Missing member variable doc comment
  84 | ERROR | Class property $comment_count_new should use lowerCamel naming without underscores
  84 | ERROR | Missing member variable doc comment
  85 | ERROR | Class property $comment_count should use lowerCamel naming without underscores
  85 | ERROR | Missing member variable doc comment
  86 | ERROR | Missing member variable doc comment
  87 | ERROR | Missing member variable doc comment
  88 | ERROR | Class property $book_ancestors should use lowerCamel naming without underscores
  88 | ERROR | Missing member variable doc comment
  90 | ERROR | Missing short description in doc comment
  91 | ERROR | Missing parameter comment
 178 | ERROR | Missing short description in doc comment
 179 | ERROR | Missing parameter comment
 179 | ERROR | Missing parameter type
-----------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/src/DrupalOrg/Taxonomy/MaintenanceStatus.php
-----------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 3 LINES
-----------------------------------------------------------------------------------------
 19 | ERROR | Missing short description in doc comment
 20 | ERROR | Missing parameter comment
 20 | ERROR | Missing parameter type
 22 | ERROR | Description for the @return value is missing
-----------------------------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/src/DrupalOrg/Taxonomy/DevelopmentStatus.php
-----------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 3 LINES
-----------------------------------------------------------------------------------------
 18 | ERROR | Missing short description in doc comment
 19 | ERROR | Missing parameter comment
 19 | ERROR | Missing parameter type
 21 | ERROR | Description for the @return value is missing
-----------------------------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/src/DrupalOrg/DrupalOrgReleases.php
--------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
--------------------------------------------------------------------------------
 15 | ERROR | Missing short description in doc comment
 16 | ERROR | Missing parameter comment
 18 | ERROR | Type hint "array" missing for $releases
--------------------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/src/DrupalOrg/DrupalOrgProjects.php
--------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
--------------------------------------------------------------------------------
 15 | ERROR | Missing short description in doc comment
 16 | ERROR | Missing parameter comment
 18 | ERROR | Type hint "array" missing for $projects
--------------------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/src/DrupalOrg/DrupalOrgClient.php
------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 3 LINES
------------------------------------------------------------------------------
 69 | ERROR | Parameter tags must be defined first in a doc comment
 69 | ERROR | Missing parameter comment
 71 | ERROR | Description for the @return value is missing
 77 | ERROR | Type hint "array" missing for $query
------------------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/src/DrupalOrg/DrupalOrgRelease.php
--------------------------------------------------------------------------------------------------
FOUND 19 ERRORS AFFECTING 14 LINES
--------------------------------------------------------------------------------------------------
 11 | ERROR | Missing member variable doc comment
 12 | ERROR | Class property $release_link should use lowerCamel naming without underscores
 12 | ERROR | Missing member variable doc comment
 13 | ERROR | Missing member variable doc comment
 14 | ERROR | Class property $date_ago should use lowerCamel naming without underscores
 14 | ERROR | Missing member variable doc comment
 15 | ERROR | Class property $is_compatible should use lowerCamel naming without underscores
 15 | ERROR | Missing member variable doc comment
 17 | ERROR | Missing member variable doc comment
 18 | ERROR | Missing member variable doc comment
 19 | ERROR | Class property $download_link should use lowerCamel naming without underscores
 19 | ERROR | Missing member variable doc comment
 20 | ERROR | Missing member variable doc comment
 21 | ERROR | Missing member variable doc comment
 22 | ERROR | Missing member variable doc comment
 23 | ERROR | Class property $core_compatibility should use lowerCamel naming without underscores
 23 | ERROR | Missing member variable doc comment
 25 | ERROR | Missing short description in doc comment
 26 | ERROR | Missing parameter comment
--------------------------------------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/src/Controller/BrowserController.php
---------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
---------------------------------------------------------------------------------
 43 | ERROR | Missing short description in doc comment
 44 | ERROR | Description for the @return value is missing
---------------------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/src/Controller/DrupalOrgProxyController.php
----------------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
----------------------------------------------------------------------------------------
  23 | ERROR | Missing short description in doc comment
  31 | ERROR | Missing parameter comment
  51 | ERROR | Missing parameter comment
  53 | ERROR | Description for the @return value is missing
 111 | ERROR | Missing parameter comment
----------------------------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/sveltejs/public/build/bundle.css
-----------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------
 1 | WARNING | File appears to be minified and cannot be processed
-----------------------------------------------------------------------------

Time: 246ms; Memory: 10MB

phpcs --standard=DrupalPractice

phpcs --standard=DrupalPractice --extensions=php,inc,install,test,profile,theme,css,info,txt,md,yml /var/www/html/products/project_browser/ --ignore="/var/www/html/products/project_browser/node_modules/ , /var/www/html/products/project_browser/sveltejs/node_modules/"

FILE: /var/www/html/products/project_browser/modules/project_browser_extender/project_browser_extender.info.yml
---------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------
 6 | WARNING | All dependencies must be prefixed with the project name, for example "drupal:"
---------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/src/DrupalOrg/DrupalOrgProject.php
-----------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------------------------
 102 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t()
     |         | instead
 159 | WARNING | Unused variable $version.
-----------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/src/DrupalOrg/DrupalOrgRelease.php
-----------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------
 41 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t()
    |         | instead
-----------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/src/Controller/BrowserController.php
---------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
---------------------------------------------------------------------------------------------
 24 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 48 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
---------------------------------------------------------------------------------------------


FILE: /var/www/html/products/project_browser/sveltejs/public/build/bundle.css
-----------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------
 1 | WARNING | File appears to be minified and cannot be processed
-----------------------------------------------------------------------------

Time: 151ms; Memory: 6MB

Comment #13

rajab natshah

he/him

commented 17 November 2021 at 10:13

#2770065: array_key_exists() micro-optimization

array_key_exists was used in number of places

one of them

    if (isset($release['terms']) || array_key_exists('terms', $release)) {
      $this->terms = $release['terms'];
    }
    $this->security = $release['security'];
    if (isset($release['core_compatibility']) || array_key_exists('core_compatibility', $release)) {
      $this->coreCompatibility = $release['core_compatibility'];
      $this->isCompatible = Semver::satisfies(\Drupal::VERSION, $this->coreCompatibility);
    }

Comment #14

rajab natshah

he/him

commented 17 November 2021 at 15:11

Status	File	Size
new	3238572-14.patch	33.99 KB

Comment #15

rajab natshah

he/him

commented 17 November 2021 at 15:15

Status:

Active

» Needs review

phpcs --standard=Drupal

phpcs --standard=Drupal --extensions=php,inc,install,test,profile,theme,css,info,txt,md,yml /var/www/html/products/project_browser/ --ignore="/var/www/html/products/project_browser/node_modules/ , /var/www/html/products/project_browser/sveltejs/node_modules/, /var/www/html/products/project_browser/sveltejs/public/build/bundle.css"

phpcs --standard=DrupalPractice

phpcs --standard=DrupalPractice --extensions=php,inc,install,test,profile,theme,css,info,txt,md,yml /var/www/html/products/project_browser/ --ignore="/var/www/html/products/project_browser/node_modules/ , /var/www/html/products/project_browser/sveltejs/node_modules/, /var/www/html/products/project_browser/sveltejs/public/build/bundle.css"

Comment #16

rajab natshah

he/him

commented 17 November 2021 at 16:31

It was hard to be sure that the right @var type is been used for the following properties in the

DrupalOrgProject.php

class


  /**
   * Author.
   *
   * @var array
   */
  public $author = [];

  /**
   * Body.
   *
   * @var array
   */
  public $body = [];

  /**
   * Created.
   *
   * @var string
   */
  public $created;

  /**
   * Changed.
   *
   * @var string
   */
  public $changed;

  /**
   * Changed Ago.
   *
   * @var string
   */
  public $changedAgo;

  /**
   * Field Project images.
   *
   * @var array
   */
  public $fieldProjectImages = [];

  /**
   * Node id.
   *
   * @var int
   */
  public $nid;

  /**
   * Status.
   *
   * @var string
   */
  public $status;

  /**
   * Type.
   *
   * @var string
   */
  public $type;

  /**
   * Title.
   *
   * @var string
   */
  public $title;

  /**
   * URL.
   *
   * @var string
   */
  public $url;

  /**
   * Project usage.
   *
   * @var array
   */
  public $projectUsage = [];

  /**
   * Project usage total.
   *
   * @var int
   */
  public $projectUsageTotal = 0;

  /**
   * Field security advisory coverage.
   *
   * @var array
   */
  public $fieldSecurityAdvisoryCoverage;

  /**
   * Field project machine name.
   *
   * @var array
   */
  public $fieldProjectMachineName;

  /**
   * Field Supporting Organizations.
   *
   * @var array
   */
  public $fieldSupportingOrganizations = [];

  /**
   * Flag Project Star User Count.
   *
   * @var int
   */
  public $flagProjectStarUserCount;

  /**
   * Module categories.
   *
   * @var string
   */
  public $taxonomyVocabulary3;

  /**
   * Maintenance status.
   *
   * @var string
   */
  public $taxonomyVocabulary44;

  /**
   * Development status.
   *
   * @var string
   */
  public $taxonomyVocabulary46;

  /**
   * Sticky.
   *
   * @var bool
   */
  protected $sticky;

  /**
   * Promote.
   *
   * @var string
   */
  protected $promote;

  /**
   * Edit URL.
   *
   * @var string
   */
  protected $editUrl;


  /**
   * Language.
   *
   * @var string
   */
  protected $language;

  /**
   * Is New.
   *
   * @var bool
   */
  protected $isNew;

  /**
   * Version id.
   *
   * @var int
   */
  protected $vid;

  /**
   * Flag project star user.
   *
   * @var string
   */
  protected $flagProjectStarUser;

  /**
   * Field issue summary template.
   *
   * @var array
   */
  protected $fieldIssueSummaryTemplate;

  /**
   * Field replaced by.
   *
   * @var array
   */
  protected $fieldReplacedBy;

  /**
   * Field next major version info.
   *
   * @var array
   */
  protected $fieldNextMajorVersionInfo;

  /**
   * Field project ecosystem.
   *
   * @var array
   */
  protected $fieldProjectEcosystem;

  /**
   * Field project issue version opts.
   *
   * @var array
   */
  protected $fieldProjectIssueVersionOpts;

  /**
   * Field project components.
   *
   * @var array
   */
  protected $fieldProjectComponents = [];

  /**
   * Field project docs.
   *
   * @var array
   */
  protected $fieldProjectDocs;

  /**
   * Field project license.
   *
   * @var array
   */
  protected $fieldProjectLicense;

  /**
   * Field project screenshots.
   *
   * @var array
   */
  protected $fieldProjectScreenshots = [];

  /**
   * Field project documentation.
   *
   * @var array
   */
  protected $fieldProjectDocumentation;

  /**
   * Field project demo.
   *
   * @var array
   */
  protected $fieldProjectDemo;

  /**
   * Upload.
   *
   * @var array
   */
  protected $upload = [];

  /**
   * Field project changelog.
   *
   * @var array
   */
  protected $fieldProjectChangelog;

  /**
   * Field project homepage.
   *
   * @var array
   */
  protected $fieldProjectHomepage;

  /**
   * Field release version format.
   *
   * @var array
   */
  protected $fieldReleaseVersionFormat;

  /**
   * Field project has releases.
   *
   * @var array
   */
  protected $fieldProjectHasReleases;

  /**
   * Field project issue guidelines.
   *
   * @var array
   */
  protected $fieldProjectIssueGuidelines;

  /**
   * Field project default component.
   *
   * @var array
   */
  protected $fieldProjectDefaultComponent;

  /**
   * Field project has issue queue.
   *
   * @var array
   */
  protected $fieldProjectHasIssueQueue;

  /**
   * Field project type.
   *
   * @var array
   */
  protected $fieldProjectType;

  /**
   * Last comment timestamp.
   *
   * @var string
   */
  protected $lastCommentTimestamp;

  /**
   * Has new content.
   *
   * @var string
   */
  protected $hasNewContent;

  /**
   * Flag tracker follower count.
   *
   * @var string
   */
  protected $flagTrackerFollowerCount;

  /**
   * Feed node id.
   *
   * @var string
   */
  protected $feedNid;

  /**
   * Feeds item url.
   *
   * @var string
   */
  protected $feedsItemUrl;

  /**
   * Feeds item guid.
   *
   * @var string
   */
  protected $feedsItemGuid;

  /**
   * Comment count new.
   *
   * @var string
   */
  protected $commentCountNew;

  /**
   * Comment count.
   *
   * @var string
   */
  protected $commentCount;

  /**
   * Comments.
   *
   * @var string
   */
  protected $comments;

  /**
   * Comment.
   *
   * @var string
   */
  protected $comment;

Comment #17

tim.plunkett

he/him

English

Philadelphia

commented 17 November 2021 at 16:42

I'd recommend waiting on this until after out hackathon this week. Lots of code changes in MRs that should be merged before this

Comment #18

rajab natshah

he/him

commented 17 November 2021 at 16:56

Status:

Needs review

» Postponed

Noted;
Postponed to after the hackathon this week.
I will send a new MR for phpcs fixes after that.
Anyone can do that too.

Comment #19

rajab natshah

he/him

commented 17 November 2021 at 16:57

Assigned:

rajab natshah

» Unassigned

Comment #20

tim.plunkett

he/him

English

Philadelphia

commented 17 November 2021 at 17:27

Also I should note: I currently have the drupalci.yml set up to copy core's code checks. Core doesn't enforce full Drupal/DrupalPractice standards yet. We can of course work on improving this, but we have the bare minimum in place now.

Comment #21

rajab natshah

he/him

commented 17 November 2021 at 17:36

Noticed that in:
#3238574: Add and activate Automated Testing for the Project Browser module
#3249483: Run core commit checks on code
That is better

Comment #22

chrisfromredfin

he/him/his

English

Portland, Maine

commented 24 November 2021 at 15:30

Should this issue stay and be advanced, or be resolved as a duplicate then?

Comment #23

rajab natshah

he/him

commented 24 November 2021 at 16:37

Issue summary:

View changes

I'm ok with one more round after all changes
As I said in the summary

It is hard to follow the code when it's not following the Drupal standard and Drupal Practice.
And that will be harder to commit or fix with MR or a patch.
Our eyes are used to read code with Drupal standards.

Comment #24

chrisfromredfin

he/him/his

English

Portland, Maine

commented 1 December 2021 at 15:22

The main 'sprint' work is mostly in, at least in Needs Review. It would be nice to have these standards ready to go in case we need to format the open MR's... ?

So feel free to resume on this one. :)

Comment #25

rajab natshah

he/him

commented 1 December 2021 at 16:00

Assigned:	Unassigned	» rajab natshah
Status:	Postponed	» Active

Comment #26

2 December 2021 at 11:51

RajabNatshah opened merge request !20

Comment #27

rajab natshah

he/him

commented 2 December 2021 at 11:53

Status:

Active

» Needs review

Comment #28

rajab natshah

he/him

commented 2 December 2021 at 13:02

Assigned:

rajab natshah

» Unassigned

Comment #29

gaurav.kapoor commented 2 December 2021 at 14:30

Still, getting these errors when running PHPCS, are we planning to do changes in 2-3 commits?, is it still in WIP? or I am missing anything?

FILE: /Users/gauravkapoor/Desktop/Programming/Contribution/Drupal9/web/modules/contrib/project_browser/project_browser.module
-----------------------------------------------------------------------------------------------------------------------------
FOUND 11 ERRORS AND 2 WARNINGS AFFECTING 13 LINES
-----------------------------------------------------------------------------------------------------------------------------
1 | ERROR | [x] Missing file doc comment
8 | ERROR | [x] Missing function doc comment
19 | ERROR | [x] Missing function doc comment
21 | ERROR | [x] Expected one space after the comma, 0 found
24 | WARNING | [x] A comma should follow the last multiline array item. Found: ]
31 | WARNING | [x] A comma should follow the last multiline array item. Found: '_project_browser_batch_finished'
35 | ERROR | [x] Expected 1 blank line after function; 2 found
38 | ERROR | [x] Missing function doc comment
54 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected "NULL" but found "null"
85 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
86 | ERROR | [x] Expected 1 space(s) after cast statement; 0 found
90 | ERROR | [x] Expected 1 space after FOREACH keyword; 0 found
98 | ERROR | [x] Expected 1 newline at end of file; 0 found
-----------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 13 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------------

FILE: /Users/gauravkapoor/Desktop/Programming/Contribution/Drupal9/web/modules/contrib/project_browser/README.md
----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 7 WARNINGS AFFECTING 7 LINES
----------------------------------------------------------------------------------------------------------------
19 | WARNING | Line exceeds 80 characters; contains 130 characters
60 | WARNING | Line exceeds 80 characters; contains 90 characters
61 | WARNING | Line exceeds 80 characters; contains 125 characters
62 | WARNING | Line exceeds 80 characters; contains 118 characters
64 | WARNING | Line exceeds 80 characters; contains 118 characters
65 | WARNING | Line exceeds 80 characters; contains 116 characters
66 | WARNING | Line exceeds 80 characters; contains 130 characters
----------------------------------------------------------------------------------------------------------------

Time: 625ms; Memory: 10MB

Comment #30

rajab natshah

he/him

commented 2 December 2021 at 14:57

In the older patch, I did them as in #16
They did not have the right @var type tho.

But in the newer MR
Only kept JSON property mapping in the DrupalOrgProject class as is.
Followed with the list of fields and their types from the following link
https://www.drupal.org/api-d7/node.json?type=project_module

Types are:

string
bool
array
Many custom AbstractData ( Not sure if we are going to restrict the type objects at this phase )

The data for some fields needs custom AbstractData classes which is not very clear to follow.

Comment #31

rajab natshah

he/him

commented 8 December 2021 at 16:16

Status:

Needs review

» Active

Comment #32

30 June 2022 at 13:49

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

Comment #33

beatrizramos commented 30 June 2022 at 13:53

Status:

Active

» Needs work

Needs work on @var type

Comment #34

fjgarlin commented 1 July 2022 at 09:12

This has evolved a lot since this issue was created. We are checking already coding standards and a few other things, so maybe we need to update the description of the issue first and see if it's still valid?

Comment #35

elber

Portuguese, Brazil

Brazil

commented 8 July 2022 at 12:39

Status	File	Size
new	Captura de tela de 2022-07-08 09-38-42.png	135.68 KB

Hi I couldn't to apply the changes made on the MR file, I followed this document: https://www.drupal.org/docs/develop/git/using-git-to-contribute-to-drupa... (see "Downloading a patch file from a merge request" section).

Comment #36

tim.plunkett

he/him

English

Philadelphia

commented 8 July 2022 at 13:28

The module is now following the coding standards set by Drupal Core, and it enforced at the testbot level via drupalci.yml.
I feel that is sufficient, and this issue should be closed.

Comment #37

elber

Portuguese, Brazil

Brazil

commented 8 July 2022 at 14:34

Hi @tim plunkett were you able to apply the changes?

Comment #38

8 July 2022 at 16:55

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

Comment #39

8 July 2022 at 16:59

bnjmnm opened merge request !193

Comment #40

bnjmnm

he/him

English

Ann Arbor, MI

commented 8 July 2022 at 17:17

Status:

Needs work

» Closed (works as designed)

The testbot checks code standards on every file that is changed in a given MR, so I created the jul_2022_phpcs, which changes a testbot setting so EVERY php file is checked.
Just to be 100% sure PHPCS is currently passing, I created the jul_2022_phpcs branch, which forces drupalci to test every PHP file.
It checked a ton of files and responded with a reassuring

13:05:27 
13:05:27 
13:05:27 Time: 2 mins, 48.93 secs; Memory: 234MB
13:05:27 
13:05:27 
13:05:27 PHPCS: passed
13:05:27

The code standards currently meet the drupalci testbot requirements, so we're good!

@elber there's no need to apply an MR as a patch, but I can see how the docs might lead you to believe that. You'll find it much easier to go to the "Issue Fork" area just below the issue summary, click "show commands" and copy/paste the commands to check out the branch that corresponds to the MR. Patch support still exists for legacy reasons, but it's not an everyday part of the Gitlab workflow.

It makes sense it wouldn't apply as it's based on a version of Project Browser from 7 months ago. This is a fast moving project, and many of the files that were changed no longer exist!