GitLab CI reports PHP_CodeSniffer warnings/errors. The ones to still fix are the following ones.

FILE: ...8347/web/modules/custom/status_dashboard-3358347/src/Form/SettingsForm.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 33 | ERROR | [x] Multi-line function declarations must have a trailing comma
    |       |     after the last parameter
    |       |     (Drupal.Functions.MultiLineFunctionDeclaration.MissingTrailingComma)
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------
Time: 242ms; Memory: 6MB

 
 

CommentFileSizeAuthor
#8 changes_coding_standard-3358347-8.patch1.54 KBrohit.rawat619

Issue fork status_dashboard-3358347

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

paraderojether created an issue. See original summary.

paraderojether opened merge request !6

paraderojether’s picture

paraderojether commented
Assigned: paraderojether » Unassigned
Status: Active » Needs review

Created an MR to fix the issues reported by phpcs.

Please review.
Thank You.

Anmol_Specbee’s picture

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

The patch is resolving all the above-mentioned issues. Moving to RTBC.

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented
Version: 2.0.x-dev » 2.x-dev
Status: Reviewed & tested by the community » Needs work

This was clearly not tested, nor reviewed. Removing issue credits as well. Also make sure to use the correct branch.

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented
Version: 2.x-dev » 2.0.x-dev

Branch was actually correct, sorry for that 🙃

rohit.rawat619’s picture

rohit.rawat619 commented
Assigned: Unassigned » rohit.rawat619
Issue summary: View changes
Status: Needs work » Active
rohit.rawat619’s picture

rohit.rawat619 commented
Assigned: rohit.rawat619 » Unassigned
Status: Active » Needs review
StatusFileSize
new changes_coding_standard-3358347-8.patch1.54 KB
schillerm’s picture

schillerm commented

Hi, I patched (#8) the 2.0.x-dev version of this module (on a D9 site) and ran the following phpcs command on it..

phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml web/modules/contrib/status_dashboard

I got the following output back.. 

FILE: /home/user/Documents/Sites/D9/web/modules/contrib/status_dashboard/status_dashboard.module
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 47 | ERROR | All functions defined in a module file must be prefixed with the module's name, found "_dashboard_email_reminder_notification_cron" but expected
    |       | "status_dashboard__dashboard_email_reminder_notification_cron"
 82 | ERROR | All functions defined in a module file must be prefixed with the module's name, found "_dashboard_module_send_notification_mail" but expected
    |       | "status_dashboard__dashboard_module_send_notification_mail"
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: /home/user/Documents/Sites/D9/web/modules/contrib/status_dashboard/status_dashboard.install
-------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------
 144 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
-------------------------------------------------------------------------------------------------------------------


FILE: /home/user/Documents/Sites/D9/web/modules/contrib/status_dashboard/src/ClientSiteInterface.php
----------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------
 61 | ERROR | Missing parameter type
----------------------------------------------------------------------------------------------------------


FILE: /home/user/Documents/Sites/D9/web/modules/contrib/status_dashboard/README.md
----------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------------------------
 61 | WARNING | Line exceeds 80 characters; contains 83 characters
 67 | WARNING | Line exceeds 80 characters; contains 92 characters
----------------------------------------------------------------------------------------


FILE: /home/user/Documents/Sites/D9/web/modules/contrib/status_dashboard/PATCHES.txt
------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
------------------------------------------------------------------------------------------
 1 | WARNING | [ ] Line exceeds 80 characters; contains 104 characters
 5 | ERROR   | [x] Expected 1 newline at end of file; 3 found
------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------

Time: 156ms; Memory: 10MB
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work

The last patch changes just two files, while the report shows errors/warnings for eight files.
Also, since there is already a MR, it should be better to keep using that MR, instead of submitting patches or creating new MRs.

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

shalini_jha’s picture

shalini_jha commented
Status: Needs work » Needs review
schillerm’s picture

schillerm commented

Hi again, back to review the latest version of MR!6.

I ran ..

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml web/modules/contrib/status_dashboard

and got nothing back.

+1 for RTBTC from me.

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented
Status: Needs review » Needs work

None of the remarks on the merge request are solved. This MR will break the module...

Removing credits once again.

nitin_lama’s picture

nitin_lama
he/him
Location India
commented
Assigned: Unassigned » nitin_lama
nitin_lama’s picture

nitin_lama
he/him
Location India
commented

For the mixed datatype i'm not sure what type status_errors variable is. Please review the MR.

nitin_lama’s picture

nitin_lama
he/him
Location India
commented
Assigned: nitin_lama » Unassigned
zkhan.aamir’s picture

zkhan.aamir commented
Status: Needs work » Needs review
bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented
Status: Needs review » Needs work

There are still private functions being made public.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Fix this issues reported by phpcs. » Fix the issues reported by phpcs
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Needs review

I think I corrected what needed to be corrected.

a.aaronjake’s picture

a.aaronjake commented
Status: Needs review » Needs work

Hi @apaderno,

Applied latest MR !16 version successfully, however it resulted to some issues.

status_dashboard git:(2.0.x) curl https://git.drupalcode.org/project/status_dashboard/-/merge_requests/6.diff | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  8558    0  8558    0     0  12264      0 --:--:-- --:--:-- --:--:-- 12367
patching file README.md
patching file src/ClientSiteInterface.php
patching file src/Collector.php
patching file src/Entity/ClientSite.php
patching file src/Form/DashboardForm.php
patching file status_dashboard.install
patching file status_dashboard.module
➜  status_dashboard git:(2.0.x) ✗ cd ..
➜  contrib git:(main) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig status_dashboard

FILE: ...al-orgissue/web/modules/contrib/status_dashboard/src/Form/SettingsForm.php
--------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------
 31 | ERROR | [x] The first parameter of a multi-line function declaration must
    |       |     be on the line after the opening bracket
 32 | ERROR | [x] Multi-line function declaration not indented correctly;
    |       |     expected 4 spaces but found 30
 32 | ERROR | [x] Multi-line function declarations must have a trailing comma
    |       |     after the last parameter
 32 | ERROR | [x] The closing parenthesis of a multi-line function declaration
    |       |     must be on a new line
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

Time: 604ms; Memory: 12MB

Kindly check

Thanks,
Jake

silvi.addweb made their first commit to this issue’s fork.

silvi.addweb’s picture

silvi.addweb commented
Status: Needs work » Needs review

Hi, I have fixed mention issue from #22. Please review

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue summary: View changes
Status: Needs review » Needs work
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Needs review
farid.muborakshoev’s picture

farid.muborakshoev commented
Status: Needs review » Fixed

Added to new release, thank you.

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented

Fix issue credits.

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented

Actually nothing was merged or pushed, and the new release is identical to the last one.

Status: Fixed » Closed (fixed)

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