Problem/Motivation
Steps to reproduce
Running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig shows the following warnings/errors, which should be fixed.
Result:
FILE: ...toolbar/admin_toolbar_tools/src/Plugin/Derivative/ExtraLinks.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
6 | 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: ...toolbar/admin_toolbar_tools/src/Controller/ToolbarController.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
17 | ERROR | [x] Use statements should be sorted alphabetically. The
| | first wrong one is
| | Drupal\Core\Template\TwigEnvironment.
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: ...m/admin_toolbar/admin_toolbar_search/admin_toolbar_search.module
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
9 | ERROR | [x] Use statements should be sorted alphabetically. The
| | first wrong one is
| | Drupal\Core\Routing\RouteMatchInterface.
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: /app/web/modules/custom/admin_toolbar/admin_toolbar.module
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
11 | ERROR | [x] Use statements should be sorted alphabetically. The
| | first wrong one is Drupal\Component\Utility\Html.
60 | ERROR | [ ] All functions defined in a module file must be
| | prefixed with the module's name, found
| | "toolbar_tools_menu_navigation_links" but expected
| | "admin_toolbar_toolbar_tools_menu_navigation_links"
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: ...bar_links_access_filter/admin_toolbar_links_access_filter.module
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
11 | ERROR | [x] Use statements should be sorted alphabetically. The
| | first wrong one is
| | Drupal\Core\Routing\RouteMatchInterface.
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | Screenshot from 2024-04-04 20-11-08.png | 53.53 KB | tirupati_singh |
Issue fork admin_toolbar-3411005
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
tirupati_singh commentedI've fixed phpcs issues please review.
Comment #4
clarkssquared commentedHi
I confirmed that your MR !66 fixes most of the issues but I will retain the status to needs review because there is still a PHPCS warnings flagged in the module so that others can review and give feedback if the warnings needs to be fixed or can be ignored.
Comment #5
tirupati_singh commentedHi @clarkssquared, I've run the
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twigcommand but didn't get any mentioned warnings. Attaching the screenshot for your reference.Comment #6
silvi.addweb commentedAfter applying patch, I'm still receiving several phpcs problems.
Comment #7
tirupati_singh commented@silvi.addweb, I've fixed the phpcs issues. Please apply the patch again and verify the changes.
Comment #8
silvi.addweb commentedHii,
Yes, I've checked and all phpcs issues are resolved now.
Comment #10
rajeshreeputraChanges looks good, CI passing, can we get it merged? this will unblock the Drupal 11 compatibility issue - #3363604: Add Drupal 11 support.
Thanks!
Comment #11
dydave commentedRebased MR!66 on 3.x with gitlabci tests and required phpcs and phpstan jobs to pass.
✅ Merge request seems to complete successfully with all PHPCS and PHPSTAN validation errors fixed, see:
Back to Needs review for more reviews, testing and feedback.
Thanks!
Comment #13
dydave commentedThanks Jakob (@japerry)!
Ticket's merge request MR!66 should be completing successfully with PHPCS and PHPStan jobs passing 🟢
Everything is green except ESLint:
https://git.drupalcode.org/issue/admin_toolbar-3411005/-/pipelines/242227
Hope we could get this one in as well!
Thanks in advance!
Comment #15
japerryComment #16
japerryMade PHPstan optional as its more likely to change due to no fault of the module.