Problem/Motivation

Phpcs defaults detect things that will not be fixed.

Steps to reproduce

Proposed resolution

Exclude those rules form being ran during the gitlab CI pipeline. Add a phpcs.xml.dist

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork office_hours-3542418

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

bluegeek9 created an issue. See original summary.

bluegeek9’s picture

bluegeek9 commented
Issue summary: View changes
johnv’s picture

johnv
Primary language Dutch
commented
Status: Active » Closed (won't fix)

Yeah, I know, do not try to resolve them.
Ther are stupid rules:
- max 80 char per line
- no empty line after inline comments
- no inline comments after a code line.
- code in inline comments is regardes as text.

Unless you can tell me a way to exclude such nonsense from the phpcs jobs, i will leave it this way.

Thanks, anyway

bluegeek9’s picture

bluegeek9 commented
Title: Issues reported by phpcs » Add phpcs.xml.dist
Status: Closed (won't fix) » Active

We can add a php.xml.dist file.

Which rules do you want to exclude?

bluegeek9 changed the visibility of the branch 8.x-1.x to hidden.

bluegeek9’s picture

bluegeek9 commented
Issue summary: View changes

bluegeek9 opened merge request !67

bluegeek9’s picture

bluegeek9 commented

code in inline comments is regarded as text.

I am not sure about that one. I think code might need to be in multi line comments.

I add a phpcs.xml.dist to handle:
- max 80 char per line
- no empty line after inline comments
- no inline comments after a code line.

bluegeek9’s picture

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

  • johnv committed 8a77664e on 8.x-1.x 
    Issue #3542418 by bluegeek9: Add phpcs.xml.dist file
johnv’s picture

johnv
Primary language Dutch
commented

Let us see what happens.

johnv’s picture

johnv
Primary language Dutch
commented

wow, what a relief. The below difference may not seem so much, but it will make a big difference on my other modules, too.
I now will add other errors at some other time, if needed. Thanks.

before: https://git.drupalcode.org/project/office_hours/-/jobs/6276228

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
-----------------------------------------------------------------------
    SOURCE                                                        COUNT
-----------------------------------------------------------------------
[x] Drupal.WhiteSpace.ScopeIndent.IncorrectExact                  30
[x] Drupal.Commenting.PostStatementComment.Found                  14
[ ] Drupal.Semantics.FunctionT.NotLiteralString                   12
[ ] Drupal.Files.LineLength.TooLong                               11
[x] Drupal.Commenting.InlineComment.InvalidEndChar                9
[x] Drupal.Classes.UnusedUseStatement.UnusedUse                   8
[ ] Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps  8
[x] Drupal.Commenting.InlineComment.NotCapital                    7
[ ] Generic.CodeAnalysis.UselessOverridingMethod.Found            6
[x] Drupal.Commenting.InlineComment.SpacingAfter                  5
[ ] Drupal.Semantics.FunctionT.ConcatString                       4
[ ] Drupal.Commenting.DocComment.MissingShort                     3
[ ] Drupal.Commenting.FunctionComment.ParamMissingDefinition      2
[x] Drupal.Commenting.FunctionComment.SeePunctuation              2
[x] Drupal.Classes.ClassDeclaration.CloseBraceAfterBody           1
[ ] Drupal.Commenting.DocComment.ParamGroup                       1
[ ] Drupal.Commenting.FunctionComment.MissingReturnComment        1
[ ] Drupal.Commenting.FunctionComment.ParamNameNoMatch            1
[x] Drupal.Commenting.FunctionComment.ReturnVarName               1
[x] Drupal.Commenting.InlineComment.NoSpaceBefore                 1
[x] Drupal.Commenting.InlineComment.SpacingBefore                 1
[x] Drupal.Commenting.VariableComment.IncorrectVarType            1
[x] Squiz.WhiteSpace.FunctionSpacing.AfterLast                    1
[x] Squiz.WhiteSpace.OperatorSpacing.NoSpaceBefore                1
-----------------------------------------------------------------------
A TOTAL OF 131 SNIFF VIOLATIONS WERE FOUND IN 24 SOURCES

After: https://git.drupalcode.org/project/office_hours/-/jobs/6277048

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
-----------------------------------------------------------------------
    SOURCE                                                        COUNT
-----------------------------------------------------------------------
[x] Drupal.WhiteSpace.ScopeIndent.IncorrectExact                  30
[ ] Drupal.Semantics.FunctionT.NotLiteralString                   12
[x] Drupal.Commenting.InlineComment.InvalidEndChar                9
[x] Drupal.Classes.UnusedUseStatement.UnusedUse                   8
[ ] Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps  8
[x] Drupal.Commenting.InlineComment.NotCapital                    7
[ ] Generic.CodeAnalysis.UselessOverridingMethod.Found            6
[ ] Drupal.Semantics.FunctionT.ConcatString                       4
[ ] Drupal.Commenting.DocComment.MissingShort                     3
[ ] Drupal.Commenting.FunctionComment.ParamMissingDefinition      2
[x] Drupal.Commenting.FunctionComment.SeePunctuation              2
[x] Drupal.Classes.ClassDeclaration.CloseBraceAfterBody           1
[ ] Drupal.Commenting.DocComment.ParamGroup                       1
[ ] Drupal.Commenting.FunctionComment.MissingReturnComment        1
[ ] Drupal.Commenting.FunctionComment.ParamNameNoMatch            1
[x] Drupal.Commenting.FunctionComment.ReturnVarName               1
[x] Drupal.Commenting.InlineComment.NoSpaceBefore                 1
[x] Drupal.Commenting.InlineComment.SpacingBefore                 1
[x] Drupal.Commenting.VariableComment.IncorrectVarType            1
[x] Squiz.WhiteSpace.FunctionSpacing.AfterLast                    1
[x] Squiz.WhiteSpace.OperatorSpacing.NoSpaceBefore                1
-----------------------------------------------------------------------
A TOTAL OF 101 SNIFF VIOLATIONS WERE FOUND IN 21 SOURCES

bluegeek9 opened merge request !68

bluegeek9’s picture

bluegeek9 commented

I addressed obvious items reported by phpcs.

I left commented code alone.

Some of the Hooks have non camel casing. I can fix it but wasn't sure if the change will be welcomed.

Public method name "OfficeHoursThemeHooks::preprocess_field" is not in lowerCamel format
| | (Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps)

johnv’s picture

johnv
Primary language Dutch
commented

"Some of the Hooks have non camel casing. I can fix it but wasn't sure if the change will be welcomed."
Fixing that would be nice. Thank you.

I really appreciate your efforts. I can learn from that and build upon it!

bluegeek9 changed the visibility of the branch 3542418-phpcs-xml-dist to hidden.

bluegeek9’s picture

bluegeek9 commented

I fixed Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps and added a unit test form the office_hours.module file.

  • johnv committed f02a43e5 on 8.x-1.x 
    Issue #3542418 by bluegeek9: Add phpcs corrections

  • johnv committed 63fe1273 on 8.x-1.x 
    Issue #3542418 by bluegeek9: Add tests/src/Unit/HookModuleUnitTest.php
johnv’s picture

johnv
Primary language Dutch
commented

Thanks, committed. Let us await the jobs.

You are fast, I assume you have some tool?

bluegeek9’s picture

bluegeek9 commented

I use ddev. It includes phpcs and phpcbf.

I also work on Drupal issues everyday :)

johnv’s picture

johnv
Primary language Dutch
commented
Status: Needs review » Fixed

Thanks.

Status: Fixed » Closed (fixed)

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