- 🇮🇳India Charchil Khandelwal
Charchil Khandelwal → made their first commit to this issue’s fork.
- 🇵🇭Philippines roberttabigue
Hi @sahil.goyal
The errors still exist after applying Patch #2 to the Drupal Mega Menu module against the 8.x-1.13 version and with Drupal core 9.5.6.
Please see the attached screenshot.
Thanks.
- Assigned to Kaustab_Roy
- Issue was unassigned.
- 🇮🇹Italy apaderno Brescia, 🇮🇹
Issues should be assigned to somebody who creates a patch or a MR in the next hours, not after more than 12 hours. This means blocking an issue for which somebody else could have worked on.
- 🇮🇳India Kaustab_Roy
patch #2 didn't applied cleanly, as the changed files/folders are not present.
Added Fixes DrupalPractices & few Drupal standard fixes.
Remaining fixes for Drupal standard, if someone doesn't pick up will do it in a while. - 🇮🇹Italy apaderno Brescia, 🇮🇹
The report shows warnings/errors for 9 files, but the patch is changing 18 files. Either the report is not complete or the patch changes more files than needed.
Keeps in mind that, when only the Drupal ruleset is used, the warnings/errors shown by the DrupalPractice ruleset cannot be fixed. If you want to fix also those warnings/errors, you need to show the report returned by
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml /Applications/MAMP/htdocs/9.4.x/drupal/modules/contrib/we_megamenu. - 🇮🇳India Kaustab_Roy
Thanks @apaderno for the feedback,
So should i update the issue summary since the patch addresses DrupalPractices isssues as well
OR
Just paste the report by phpcs DrupalPractices before the patrch applied in the comments , which might be long.
I wanted to know this, since i am unaware of the right procedure - 🇮🇹Italy apaderno Brescia, 🇮🇹
The report must show all the warnings/errors given by
phpcs.
Notice that:- The used ruleset is Drupal; I am not sure which is the reason for this, but for Drupal coding standards there is also DrupalPractice, which should be used too.
- The extensions list (
php,module,inc,install,test,profile,theme,css,info,txt,md,yml) does not include thejsextension, which should be present.
- 🇮🇳India Kaustab_Roy
updated the issue summary with js parameter & updated report with DrupalPractices included.
upadated the steps to reproduce command by adding ignore the assets/includes & fonts folders. - 🇮🇹Italy apaderno Brescia, 🇮🇹
The last patch needs to be rerolled, since the file to change are now 60.
- Status changed to Needs review
about 1 year ago 5:17pm 18 October 2023 - 🇮🇳India adwivedi008
Worked on #13 and find the following issues using this command :
phpcs --ignore=*/assets/includes/*,*/assets/fonts/* --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,info,txt,md,yml we_megamenu/FILE: /var/www/html/Live_Project/Patches/we_megamenu/we_megamenu.install ------------------------------------------------------------------------ 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/Live_Project/Patches/we_megamenu/we_megamenu.module ------------------------------------------------------------------------------------------------------------------------------------ FOUND 67 ERRORS AND 12 WARNINGS AFFECTING 74 LINES ------------------------------------------------------------------------------------------------------------------------------------ 1 | ERROR | [x] Missing file doc comment 10 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Block\BlockPluginInterface. 147 | ERROR | [x] Expected 1 blank line after function; 2 found 150 | ERROR | [ ] Missing short description in doc comment 160 | ERROR | [x] Expected 1 space after IF keyword; 0 found 164 | ERROR | [x] Expected 1 space after IF keyword; 0 found 164 | ERROR | [x] Expected 1 space after closing parenthesis; found 2 171 | ERROR | [x] Whitespace found at end of line 175 | WARNING | [ ] Unused variable $block_config. 175 | ERROR | [x] Use null coalesce operator instead of ternary operator. 199 | ERROR | [x] Use null coalesce operator instead of ternary operator. 200 | ERROR | [x] Use null coalesce operator instead of ternary operator. 201 | ERROR | [x] Use null coalesce operator instead of ternary operator. 202 | ERROR | [x] Use null coalesce operator instead of ternary operator. 203 | ERROR | [x] Use null coalesce operator instead of ternary operator. 204 | ERROR | [x] Use null coalesce operator instead of ternary operator. 205 | ERROR | [x] Use null coalesce operator instead of ternary operator. 206 | ERROR | [x] Use null coalesce operator instead of ternary operator. 221 | ERROR | [x] Expected 1 space after IF keyword; 0 found 225 | ERROR | [x] Expected 1 space after IF keyword; 0 found 225 | ERROR | [x] Expected 1 space after closing parenthesis; found 2 232 | ERROR | [x] Whitespace found at end of line 258 | ERROR | [x] Use null coalesce operator instead of ternary operator. 259 | ERROR | [x] Use null coalesce operator instead of ternary operator. 260 | ERROR | [x] Use null coalesce operator instead of ternary operator. 261 | ERROR | [x] Use null coalesce operator instead of ternary operator. 262 | ERROR | [x] Use null coalesce operator instead of ternary operator. 263 | ERROR | [x] Use null coalesce operator instead of ternary operator. 264 | ERROR | [x] Use null coalesce operator instead of ternary operator. 265 | ERROR | [x] Use null coalesce operator instead of ternary operator. 276 | WARNING | [ ] Unused variable $data_config. 277 | WARNING | [ ] Unused variable $key_li. 285 | WARNING | [ ] Line exceeds 80 characters; contains 105 characters 297 | ERROR | [x] Inline control structures are not allowed 305 | ERROR | [x] Whitespace found at end of line 338 | ERROR | [x] Use null coalesce operator instead of ternary operator. 346 | ERROR | [x] Use null coalesce operator instead of ternary operator. 382 | ERROR | [x] Whitespace found at end of line 387 | ERROR | [x] Use null coalesce operator instead of ternary operator. 424 | WARNING | [ ] Unused variable $key_row. 447 | ERROR | [x] Use null coalesce operator instead of ternary operator. 452 | ERROR | [x] Use null coalesce operator instead of ternary operator. 454 | ERROR | [x] Use null coalesce operator instead of ternary operator. 455 | ERROR | [x] Use null coalesce operator instead of ternary operator. 462 | ERROR | [ ] Parameter comment must start with a capital letter 462 | ERROR | [x] Parameter comment must be on the next line 463 | ERROR | [ ] Doc comment for parameter $vars does not match actual variable name <undefined> 473 | WARNING | [ ] Unused variable $key_col. 526 | ERROR | [x] Whitespace found at end of line 543 | ERROR | [x] Use null coalesce operator instead of ternary operator. 545 | ERROR | [x] Use null coalesce operator instead of ternary operator. 546 | ERROR | [x] Use null coalesce operator instead of ternary operator. 547 | ERROR | [x] Use null coalesce operator instead of ternary operator. 548 | ERROR | [x] Use null coalesce operator instead of ternary operator. 549 | ERROR | [x] Use null coalesce operator instead of ternary operator. 570 | WARNING | [ ] Unused variable $key_li. 572 | WARNING | [ ] Unused variable $key_item. 591 | WARNING | [ ] Unused variable $key_li. 650 | ERROR | [x] Whitespace found at end of line 652 | ERROR | [x] Whitespace found at end of line 658 | ERROR | [x] Whitespace found at end of line 660 | ERROR | [x] Perl-style comments are not allowed; use "// Comment" instead 680 | ERROR | [x] Perl-style comments are not allowed; use "// Comment" instead 685 | WARNING | [ ] Unused variable $rows_content. 701 | ERROR | [x] Expected 1 space after "=>"; 0 found 702 | ERROR | [x] Expected 1 space after "=>"; 0 found 704 | ERROR | [x] Whitespace found at end of line 705 | WARNING | [ ] Unused variable $menu_position. 726 | ERROR | [x] Whitespace found at end of line 726 | ERROR | [x] Functions must not contain multiple empty lines in a row; found 2 empty lines 730 | ERROR | [x] Whitespace found at end of line 732 | ERROR | [x] Whitespace found at end of line 735 | ERROR | [x] Whitespace found at end of line 760 | ERROR | [x] Whitespace found at end of line 762 | ERROR | [x] Whitespace found at end of line 776 | WARNING | [ ] Unused variable $child_item. 782 | ERROR | [x] Expected 1 space after IF keyword; 0 found 787 | ERROR | [x] Whitespace found at end of line 789 | ERROR | [x] Whitespace found at end of line ------------------------------------------------------------------------------------------------------------------------------------ PHPCBF CAN FIX THE 64 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------------------------------------------------------------ FILE: /var/www/html/Live_Project/Patches/we_megamenu/src/WeMegaMenuBuilder.php ----------------------------------------------------------------------------------------------------------- FOUND 65 ERRORS AND 17 WARNINGS AFFECTING 77 LINES ----------------------------------------------------------------------------------------------------------- 7 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal. 7 | ERROR | [x] Non-namespaced classes/interfaces/traits should not be referenced with use statements 10 | ERROR | [x] Missing class doc comment 12 | ERROR | [ ] Parameter $backend is not described in comment 24 | ERROR | [x] Expected 3 space(s) before asterisk; 2 found 62 | WARNING | [ ] Unused variable $key_item. 88 | ERROR | [ ] Parameter $backend is not described in comment 100 | ERROR | [x] Expected 3 space(s) before asterisk; 2 found 114 | ERROR | [x] Expected 3 space(s) before asterisk; 2 found 167 | WARNING | [ ] Unused variable $key_menu. 176 | WARNING | [ ] Unused variable $key_item. 181 | WARNING | [ ] Unused variable $key_menu. 190 | ERROR | [x] Expected 1 blank line after function; 2 found 222 | ERROR | [x] Parameter comment must be on the next line 224 | ERROR | [x] Separate the @param and @return sections by a blank line. 280 | ERROR | [x] Parameter comment must end with a full stop 303 | ERROR | [ ] Return type "string || bool" must not contain spaces 333 | WARNING | [ ] Unused variable $result. 375 | WARNING | [ ] Unused variable $key_col. 392 | WARNING | [ ] Unused variable $key. 423 | ERROR | [x] Perl-style comments are not allowed; use "// Comment" instead 430 | ERROR | [x] Perl-style comments are not allowed; use "// Comment" instead 451 | ERROR | [x] Whitespace found at end of line 482 | WARNING | [ ] Unused variable $key_col. 489 | ERROR | [x] Expected 1 space after "="; 2 found 508 | WARNING | [ ] Unused variable $key_position. 521 | WARNING | [ ] Unused variable $key_menu_item. 522 | WARNING | [ ] Unused variable $key_mega_menu. 553 | WARNING | [ ] Unused variable $key_item. 588 | ERROR | [x] Missing function doc comment 589 | ERROR | [x] Expected 1 space after IF keyword; 0 found 594 | ERROR | [x] Expected 1 space after FOREACH keyword; 0 found 594 | WARNING | [ ] Unused variable $i. 595 | ERROR | [x] Expected 1 space after FOREACH keyword; 0 found 595 | WARNING | [ ] Unused variable $j. 602 | ERROR | [x] Missing function doc comment 605 | ERROR | [x] Expected 1 space after FOREACH keyword; 0 found 611 | ERROR | [x] Missing function doc comment 613 | WARNING | [ ] Unused variable $i. 614 | WARNING | [ ] Unused variable $j. 615 | ERROR | [x] Use null coalesce operator instead of ternary operator. 616 | WARNING | [ ] Unused variable $k. 626 | ERROR | [x] Missing function doc comment 627 | ERROR | [x] Expected 1 space after IF keyword; 0 found 630 | ERROR | [x] Expected 1 space after FOR keyword; 0 found 630 | ERROR | [x] There must not be a single space before a unary operator statement 631 | ERROR | [x] Expected 1 space after IF keyword; 0 found 638 | ERROR | [x] Missing function doc comment 661 | ERROR | [x] Whitespace found at end of line 664 | ERROR | [x] Missing function doc comment 669 | ERROR | [x] Comments may not appear after statements 673 | ERROR | [x] Missing function doc comment 684 | ERROR | [x] Missing function doc comment 689 | ERROR | [x] Expected 1 space after IF keyword; 0 found 695 | ERROR | [x] Expected 1 space after IF keyword; 0 found 698 | ERROR | [x] Expected 1 space after IF keyword; 0 found 701 | ERROR | [x] Expected 1 space after FOREACH keyword; 0 found 702 | ERROR | [x] Expected 1 space after FOREACH keyword; 0 found 703 | ERROR | [x] Use null coalesce operator instead of ternary operator. 704 | ERROR | [x] Expected 1 space after FOREACH keyword; 0 found 705 | ERROR | [x] Expected 1 space after IF keyword; 0 found 709 | ERROR | [x] Expected 1 space after IF keyword; 0 found 711 | ERROR | [x] Whitespace found at end of line 712 | ERROR | [x] Expected 1 space after IF keyword; 0 found 721 | ERROR | [x] There must not be a single space before a unary operator statement 725 | ERROR | [x] Whitespace found at end of line 738 | ERROR | [x] Use null coalesce operator instead of ternary operator. 748 | ERROR | [x] Expected 1 space after IF keyword; 0 found 749 | ERROR | [x] Expected 1 space after IF keyword; 0 found 765 | ERROR | [x] There must not be a single space before a unary operator statement 771 | ERROR | [x] Expected 1 space after IF keyword; 0 found 772 | ERROR | [x] There must not be a single space before a unary operator statement 775 | ERROR | [x] Expected 1 space after IF keyword; 0 found 781 | ERROR | [x] Expected 1 space after FOR keyword; 0 found 781 | ERROR | [x] There must not be a single space before a unary operator statement 785 | ERROR | [x] Expected 1 space after IF keyword; 0 found 796 | ERROR | [x] Missing function doc comment 798 | ERROR | [x] Expected 1 space after FOREACH keyword; 0 found 800 | ERROR | [x] Expected 1 space after IF keyword; 0 found 806 | ERROR | [x] Missing function doc comment 824 | ERROR | [x] Expected 1 blank line after function; 0 found 825 | ERROR | [x] The closing brace for the class must have an empty line before it ----------------------------------------------------------------------------------------------------------- PHPCBF CAN FIX THE 62 MARKED SNIFF VIOLATIONS AUTOMATICALLY ----------------------------------------------------------------------------------------------------------- FILE: /var/www/html/Live_Project/Patches/we_megamenu/src/Event/AfterConfigSave.php ------------------------------------------------------------------------------------------------------------------ FOUND 15 ERRORS AFFECTING 12 LINES ------------------------------------------------------------------------------------------------------------------ 9 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph 20 | ERROR | Class property $menu_name should use lowerCamel naming without underscores 34 | ERROR | Class property $config_data should use lowerCamel naming without underscores 39 | ERROR | Missing parameter comment 39 | ERROR | Missing parameter type 40 | ERROR | Missing parameter comment 40 | ERROR | Missing parameter type 41 | ERROR | Missing parameter comment 41 | ERROR | Missing parameter type 49 | ERROR | Missing short description in doc comment 50 | ERROR | Description for the @return value is missing 56 | ERROR | Missing short description in doc comment 57 | ERROR | Description for the @return value is missing 63 | ERROR | Missing short description in doc comment 64 | ERROR | Description for the @return value is missing ------------------------------------------------------------------------------------------------------------------ FILE: /var/www/html/Live_Project/Patches/we_megamenu/src/Plugin/Block/WeMegaMenuBlock.php ----------------------------------------------------------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE ----------------------------------------------------------------------------------------------------------------------------------- 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Config\ConfigFactoryInterface. ----------------------------------------------------------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY ----------------------------------------------------------------------------------------------------------------------------------- FILE: /var/www/html/Live_Project/Patches/we_megamenu/src/Plugin/Derivative/WeMegaMenuBlock.php ---------------------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE ---------------------------------------------------------------------------------------------- 15 | ERROR | [x] Expected 1 blank line before function; 0 found ---------------------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY ---------------------------------------------------------------------------------------------- FILE: /var/www/html/Live_Project/Patches/we_megamenu/src/Controller/WeMegaMenuAdminController.php ----------------------------------------------------------------------------------------------------------------------------------- FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES ----------------------------------------------------------------------------------------------------------------------------------- 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Controller\ControllerBase. 137 | WARNING | [ ] Menu::load calls should be avoided in classes, use dependency injection instead ----------------------------------------------------------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY ----------------------------------------------------------------------------------------------------------------------------------- FILE: /var/www/html/Live_Project/Patches/we_megamenu/README.md ----------------------------------------------------------------------- FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES ----------------------------------------------------------------------- 4 | WARNING | [ ] Line exceeds 80 characters; contains 94 characters 56 | ERROR | [x] Expected 1 newline at end of file; 0 found ----------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY ----------------------------------------------------------------------- Time: 305ms; Memory: 18MBResolved all the above mentioned issues
- 🇮🇳India chaitanyadessai Goa
Fixed remaining issues.
ran command vendor/bin/phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,info,txt,md,yml web/modules/we_megamenu - Status changed to RTBC
6 months ago 5:14pm 24 May 2024 - 🇵🇭Philippines roberttabigue
Hi @chaitanyadessai,
I have reviewed the changes and confirmed that Patch was applied cleanly to the Drupal Mega Menu module against 8.x-1.x-dev on Drupal 10.
And all PHPCS errors have been fixed.
I ran this command on the module:
phpcs --standard=Drupal,DrupalPractice --extensions=php,inc,module,install,info,test,profile,theme we_megamenuPlease see the attached files for reference.
I'm moving this now to RTBC.
Thank you!
- Status changed to Needs work
6 months ago 11:00am 25 May 2024 - 🇮🇹Italy apaderno Brescia, 🇮🇹
- * @param $menu_name - * @param $theme - * @param $config_data + * @param string $menu_name + * Menu name. + * @param string $theme + * Theme. + * @param string $config_data + * Config data.All those descriptions are each missing a definite article.
/** + * To get config data. + * * @return string + * Return config data. */ public function getConfigData() { - return $this->config_data; + return $this->configData; }To get config data. is not a sufficient/correct description.
+ * To get Menu name. + * * @return string + * Return menu name.It is not necessary to start a return value description with Return.
/** + * To get theme data. + * * @return string + * Return theme */The description is also missing a period.
+ * @param bool $backend + * Prameter $backend.+ * @param bool $backend + * Prameter $backend.The parameter description is merely repeating the parameter name. That information is already given from the previous line.
- * @param string $name as router name + * @param string $name + * As router name * Public static function routeExists string name. + * * @return int * Public static function routeExists int.As router name is not a correct description, nor are the existing ones correct.
* @param string $theme - * Public static function renderWeMegaMenuBlock theme + * Public static function renderWeMegaMenuBlock theme.That description is not correct, with or without period.
- * @return string || bool + * @return string||bool * Public static function loadConfig string.Neither the type hinting nor the description is correct.
+ /** + * Get count of Megamenu SubItem. + */ public static function countMegamenuSubItem($menu_config, $derivativeId) {Assuming that is not an inherited method, the parameter and the return type descriptions are missing. The same holds true for other methods.