Problem/Motivation

Checking PHPCS with Drupal standard on core's module return:

FILE: ...dules/tracker/tests/src/Kernel/Migrate/d7/MigrateTrackerSettingsTest.php
------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------
14 | ERROR | Missing member variable doc comment
------------------------------------------------------------------------------

FILE: ...rojects/core10/core/modules/tracker/tests/src/Functional/TrackerTest.php
------------------------------------------------------------------------------
FOUND 6 ERRORS AND 2 WARNINGS AFFECTING 8 LINES
------------------------------------------------------------------------------
58 | ERROR | [x] Missing function doc comment
63 | ERROR | [ ] The array declaration extends to column 103 (the limit
| | is 80). The array content should be split up over
| | multiple lines
96 | ERROR | [ ] The array declaration extends to column 204 (the limit
| | is 80). The array content should be split up over
| | multiple lines
182 | ERROR | [ ] The array declaration extends to column 204 (the limit
| | is 80). The array content should be split up over
| | multiple lines
209 | ERROR | [ ] The array declaration extends to column 204 (the limit
| | is 80). The array content should be split up over
| | multiple lines
335 | WARNING | [x] There must be no blank line following an inline comment
338 | WARNING | [x] There must be no blank line following an inline comment
468 | ERROR | [x] Parameter comment must end with a full stop
------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------

FILE: ...re10/core/modules/tracker/tests/src/Functional/TrackerNodeAccessTest.php
------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------
36 | ERROR | [x] Missing function doc comment
------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------

FILE: /home/tra/projects/core10/core/modules/tracker/tracker.module
------------------------------------------------------------------------------
FOUND 1 ERROR AND 2 WARNINGS AFFECTING 3 LINES
------------------------------------------------------------------------------
93 | WARNING | [x] There must be no blank line following an inline comment
266 | ERROR | [ ] Type hint "\Drupal\node\NodeInterface" missing for $node
323 | WARNING | [x] There must be no blank line following an inline comment
------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------

Steps to reproduce

Just run
phpcs --standard=Drupal --extensions=module,php,yml,twig,html core/modules/tracker

Proposed resolution

Fix it

Remaining tasks

N/A

User interface changes

N/A

API changes

N/A

Data model changes

N/A

CommentFileSizeAuthor
#10 interdiff_7-10.txt427 bytesmrinalini9
#10 3342301-10.patch8.41 KBmrinalini9
#9 3342301_7-9.txt576 bytes_pratik_
#9 3342301-9.patch8.58 KB_pratik_
#7 3342301-7.patch8.12 KBakram khan
#2 3342301-phpcs.patch6.7 KBtra.duong

Issue fork tracker-3342301

Command icon 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:

About issue forks

Comments

tra.duong created an issue. See original summary.

tra.duong’s picture

tra.duong commented
StatusFileSize
new 3342301-phpcs.patch6.7 KB
tra.duong’s picture

tra.duong commented
Status: Active » Needs review

I add patch to fix the PHPCS, please review

arunkumark’s picture

arunkumark
Location Coimbatore
commented
Assigned: Unassigned » arunkumark
arunkumark’s picture

arunkumark
Location Coimbatore
commented
Status: Needs review » Needs work

The patch applied cleanly but there are few Coding standard issues.

FILE: /var/www/html/web/modules/custom/tracker/tests/src/Functional/TrackerTest.php
-----------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------
 423 | WARNING | Unused variable $unpublished.
-----------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/custom/tracker/tracker.views.inc
-----------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------
 160 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
-----------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/custom/tracker/src/Controller/TrackerController.php
------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------------
 214 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
------------------------------------------------------------------------------------------------------------------------------------------
arunkumark’s picture

arunkumark
Location Coimbatore
commented
Assigned: arunkumark » Unassigned
akram khan’s picture

akram khan
Primary language English
Location Cuttack, Odisha
commented
StatusFileSize
new 3342301-7.patch8.12 KB

Updated patch fixed remaining PHPCS error left with 1 warning that is "423 | WARNING | Unused variable $unpublished."

arunkumark’s picture

arunkumark
Location Coimbatore
commented

Applied the patch, It applies cleanly. But there is one unresolved warning on the TrackerTest.php

FILE: /var/www/html/web/modules/custom/tracker/tests/src/Functional/TrackerTest.php
-----------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------
 408 | WARNING | Unused variable $unpublished.
-----------------------------------------------------------------------------------
_pratik_’s picture

_pratik_
Primary language English
Location Indore
commented
Status: Needs work » Needs review
StatusFileSize
new 3342301-9.patch8.58 KB
new 3342301_7-9.txt576 bytes

Please review thanks.

mrinalini9’s picture

mrinalini9 commented
StatusFileSize
new 3342301-10.patch8.41 KB
new interdiff_7-10.txt427 bytes

Changes in patch #9 doesn't seems correct, why remove the node creation part?

Added patch by fixing warnings in #8, please review it.

Thanks!

arunkumark’s picture

arunkumark
Location Coimbatore
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +RTBC

The patch was applied cleanly and resolved all the PHPCS issues.

arunkumark’s picture

arunkumark
Location Coimbatore
commented

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

urvashi_vora’s picture

urvashi_vora commented

Hi,

I reviewed and tested patch #10. Worked fine for me.

Applied smoothly.

urvasi@urvasi-Inspiron-15-3552:/var/www/html/contribution/drupal8/web/modules/contrib/tracker-3342301$ git apply -v 3342301-10.patch
Checking patch src/Controller/TrackerController.php...
Checking patch tests/src/Functional/TrackerNodeAccessTest.php...
Checking patch tests/src/Functional/TrackerTest.php...
Checking patch tests/src/Kernel/Migrate/d7/MigrateTrackerSettingsTest.php...
Checking patch tracker.module...
Checking patch tracker.views.inc...
Applied patch src/Controller/TrackerController.php cleanly.
Applied patch tests/src/Functional/TrackerNodeAccessTest.php cleanly.
Applied patch tests/src/Functional/TrackerTest.php cleanly.
Applied patch tests/src/Kernel/Migrate/d7/MigrateTrackerSettingsTest.php cleanly.
Applied patch tracker.module cleanly.
Applied patch tracker.views.inc cleanly.

Thanks for the work.

Committing the changes for helping the maintainers.

urvashi_vora opened merge request !3

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Category: Bug report » Task
Priority: Normal » Minor
Issue tags: -RTBC +Coding standards
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Tracker PHPCS error » Fix the issues reported by phpcs

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

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Status: Reviewed & tested by the community » Needs review

I fixed a remaining phpcs issue. This can be tested. Pipeline passes on the phpcs step

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Title: Fix the issues reported by phpcs » Fix the issues reported by phpcs, phpstan and phpunit (gitlab CI)
Assigned: Unassigned » batigolix
Issue summary: View changes
Issue tags: -Novice

Gitlab CI was added some time ago, and this fails on phpcs, phpstan and phpunit.

I propose to fix this together in this issue, so other issues can use the pipeline for tests.

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Issue summary: View changes
batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Issue summary: View changes
batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Status: Needs review » Needs work
batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Status: Reviewed & tested by the community » Needs review

I can get the phpunit tests working locally but not in the gitlab CI. So I revert any changes that are related to phpunit.

I made one change for phpcs.

This can be reviewed: phpcs passes in the gitlab CI pipeline.

adwivedi008’s picture

adwivedi008 commented
Status: Needs review » Reviewed & tested by the community

As the issue is for PHPCS and the pipeline passes all PHPCS issue, we can move the issue to RTBC and can create another linked issue for phpunit tests

moving the issue to RTBC

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.