Running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,info,txt,md,yml,css shows the following warnings/errors which should be fixed.

FILE: ./README.txt
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
 14 | WARNING | Line exceeds 80 characters; contains 108 characters
 43 | WARNING | Line exceeds 80 characters; contains 303 characters
----------------------------------------------------------------------


FILE: ./block_list_override.install
----------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
----------------------------------------------------------------------
 37 | ERROR | [x] Spaces must be used to indent lines; tabs are not
    |       |     allowed
 38 | ERROR | [x] Spaces must be used to indent lines; tabs are not
    |       |     allowed
 39 | ERROR | [x] Spaces must be used to indent lines; tabs are not
    |       |     allowed
----------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./block_list_override.links.menu.yml
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 6 | ERROR | [x] Expected 1 newline at end of file; 2 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./config/install/block_list_override.settings.yml
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 8 | ERROR | [x] Expected 1 newline at end of file; 2 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./src/BlockListOverride.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
----------------------------------------------------------------------
 148 | ERROR | [x] Expected 1 space after IF keyword; 0 found
 148 | ERROR | [x] Expected 1 space after closing parenthesis; found
     |       |     0
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./src/Controller/DefaultController.php
----------------------------------------------------------------------
FOUND 5 ERRORS AND 1 WARNING AFFECTING 6 LINES
----------------------------------------------------------------------
   7 | ERROR   | [x] Use statements should be sorted alphabetically.
     |         |     The first wrong one is
     |         |     Drupal\Core\Block\BlockManagerInterface.
  14 | WARNING | [ ] The class short comment should describe what the
     |         |     class does and not simply repeat the class name
  49 | ERROR   | [x] Parameter comment indentation must be 3 spaces,
     |         |     found 2 spaces
  51 | ERROR   | [x] Parameter comment indentation must be 3 spaces,
     |         |     found 2 spaces
 129 | ERROR   | [x] Expected 1 blank line after function; 0 found
 130 | ERROR   | [x] The closing brace for the class must have an
     |         |     empty line before it
----------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./src/Controller/LayoutController.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 5 | WARNING | [x] Unused use statement
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./src/Form/SettingsForm.php
----------------------------------------------------------------------
FOUND 16 ERRORS AND 16 WARNINGS AFFECTING 18 LINES
----------------------------------------------------------------------
   7 | ERROR   | [x] Use statements should be sorted alphabetically.
     |         |     The first wrong one is
     |         |     Drupal\Core\Config\ConfigFactoryInterface.
  13 | WARNING | [ ] The class short comment should describe what the
     |         |     class does and not simply repeat the class name
  31 | ERROR   | [x] Missing function doc comment
  59 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
  59 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
  71 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
  71 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
  84 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
  84 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
  89 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
  89 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
  96 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
  96 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
  99 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
  99 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
 105 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
 105 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
 124 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
 124 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
 140 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
 140 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
 147 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
 147 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
 161 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
 161 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
 171 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
 171 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
 178 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
 178 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
 185 | WARNING | [ ] Translatable strings must not begin or end with
     |         |     white spaces, use placeholders with t() for
     |         |     variables
 185 | ERROR   | [ ] Concatenating translatable strings is not
     |         |     allowed, use placeholders instead and only one
     |         |     string literal
 196 | WARNING | [ ] Possible useless method overriding detected
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./src/LayoutPluginAlter.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
  8 | ERROR | [x] Expected 1 space before opening brace; found 0
 24 | ERROR | [x] Expected 1 newline at end of file; 2 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./src/PluginAlter.php
----------------------------------------------------------------------
FOUND 4 ERRORS AND 1 WARNING AFFECTING 5 LINES
----------------------------------------------------------------------
   8 | ERROR   | [x] Use statements should be sorted alphabetically.
     |         |     The first wrong one is
     |         |     Drupal\Core\Config\ConfigFactoryInterface.
   9 | WARNING | [x] Unused use statement
  37 | ERROR   | [ ] Parameter $config_factory is not described in
     |         |     comment
  40 | ERROR   | [ ] Doc comment for parameter $configFactory does
     |         |     not match actual variable name $list_service
 100 | ERROR   | [x] Expected 1 newline at end of file; 2 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./tests/src/Functional/BlockListOverrideTest.php
----------------------------------------------------------------------
FOUND 10 ERRORS AND 1 WARNING AFFECTING 7 LINES
----------------------------------------------------------------------
  34 | ERROR   | [ ] Missing @var tag in member variable comment
  70 | ERROR   | [x] Expected 3 space(s) before asterisk; 4 found
  71 | ERROR   | [x] Expected 3 space(s) before asterisk; 4 found
  71 | ERROR   | [ ] Missing @var tag in member variable comment
  86 | WARNING | [ ] Line exceeds 80 characters; contains 82
     |         |     characters
 145 | ERROR   | [x] No space found before comment text; expected "//
     |         |     sleep(30);" but found "//sleep(30);"
 145 | ERROR   | [x] Inline comments must end in full-stops,
     |         |     exclamation marks, question marks, colons, or
     |         |     closing parentheses
 200 | ERROR   | [x] No space found before comment text; expected "//
     |         |     sleep(30);" but found "//sleep(30);"
 200 | ERROR   | [x] Inline comments must end in full-stops,
     |         |     exclamation marks, question marks, colons, or
     |         |     closing parentheses
 254 | ERROR   | [x] No space found before comment text; expected "//
     |         |     sleep(30);" but found "//sleep(30);"
 254 | ERROR   | [x] Inline comments must end in full-stops,
     |         |     exclamation marks, question marks, colons, or
     |         |     closing parentheses
----------------------------------------------------------------------
PHPCBF CAN FIX THE 8 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 107ms; Memory: 10MB
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:

Comments

jayesh_makwana created an issue. See original summary.

suresh prabhu parkala’s picture

Assigned: Unassigned » suresh prabhu parkala
suresh prabhu parkala’s picture

Status: Active » Needs review
StatusFileSize
new14.82 KB

Please review the patch.

avpaderno’s picture

Title: Coding Standard Issue » Rewrite the code to follow the coding standards
Version: 1.0.0 » 1.0.x-dev
Assigned: suresh prabhu parkala » Unassigned
Category: Bug report » Task
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: -Coding standard issue +Coding standards
-      //sleep(30);
+      // sleep(30);.

Code isn't a sentence; it doesn't need to end with a period. I would rather remove commented out code, especially when that is debugging code.

kartiktandon’s picture

Assigned: Unassigned » kartiktandon
ayush.khare’s picture

Status: Needs work » Needs review
StatusFileSize
new15.13 KB
new8.08 KB

Added reroll for #3 as it doesn't apply.

ayush.khare’s picture

Assigned: kartiktandon » Unassigned
StatusFileSize
new15.07 KB
new1.23 KB

Removed commented out code as mentioned in #4.

kartiktandon’s picture

StatusFileSize
new104.88 KB

@ayush.khare Please read the Drupal guidelines carefully. As mentioned to avoid duplication of effects issues are assigned to an individual. Attaching a Screenshot for your reference.
I already did the work for this patch and about to post the patch. But saw your comment with the patch. So please avoid this type of Approach for Drupal Contributions or towards Drupal Community.

avpaderno’s picture

Actually, the first point on Assigning ownership of issues / Assigning ownership of other project issues uses the following sentence. (Emphasis is mine.)

Assigning ownership can be useful when an issue is limited to a single task and that task might take some time to complete.

It doesn't take a week to change what reported in comment #4.

avpaderno’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update
 /**
- * Class SettingsForm.
+ * Class of SettingsForm.
  */

The comment is just reporting the class name. That is not what Drupal coding standards say a documentation comment should say for a class, but simply adding of between Class and the class name is not correct.

Then, there are other documentation comments that should be changed too, like the following one.

 /**
  * Class DefaultController.
  */

The issue summary needs to be updated to make clear exactly what needs to be changed as per coding standards. If the issue is about fixing what phpcs reports, the issue summary should show which arguments are used for phpcs and what the command exactly reports.
In this way, the project maintainers can verify the patch is correct, and every user will be able to provide a new patch if the previous one needs work or becomes outdated.

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

vishaljd’s picture

Assigned: Unassigned » vishaljd

vishaljd’s picture

Assigned: vishaljd » Unassigned
Status: Needs work » Needs review

Fixed coding standards using phpcs for Drupal and DrupalPractice standards.

Please check and merge pr.

Thanks

avpaderno’s picture

Status: Needs review » Needs work

See my previous comment. The issue summary has not been updated.

avpaderno’s picture

The MR does not correctly change the files.

 /**
- * Class DefaultController.
+ * Class of DefaultController.
  */

That is not what a documentation comment for a class should say. Adding of is not sufficient.

-  /**
-   * {@inheritdoc}
-   */
-  public function validateForm(array &$form, FormStateInterface $form_state) {
-    parent::validateForm($form, $form_state);
-  }
-

That change is not required by the coding standards.

-    //sleep(30);
+    // sleep(30);.

There is no need to add a period to commented out code.

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

_pratik_’s picture

Status: Needs work » Needs review
avpaderno’s picture

Status: Needs review » Needs work

The issue summary still needs to be updated.

avpaderno’s picture

As for the MR, it still uses Class of.

zkhan.aamir’s picture

Title: Rewrite the code to follow the coding standards » Fix the issues reported by phpcs
Issue summary: View changes
Issue tags: -Needs issue summary update

Issue summary Updated.
Please review.

Shreyas gowda’s picture

StatusFileSize
new7.47 KB

resolved some of this error
still few warnings and errors are left

FILE: C:\wamp64\www\contribution\block_list_override\README.txt
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
14 | WARNING | Line exceeds 80 characters; contains 108 characters
43 | WARNING | Line exceeds 80 characters; contains 303 characters
----------------------------------------------------------------------

FILE: C:\wamp64\www\contribution\block_list_override\src\Controller\DefaultController.php
-----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------
14 | WARNING | The class short comment should describe what the class does and not simply repeat the class name
-----------------------------------------------------------------------------------------------------------------

FILE: C:\wamp64\www\contribution\block_list_override\src\Form\SettingsForm.php
-------------------------------------------------------------------------------------------------------------------------
FOUND 14 ERRORS AND 16 WARNINGS AFFECTING 16 LINES
-------------------------------------------------------------------------------------------------------------------------
13 | WARNING | The class short comment should describe what the class does and not simply repeat the class name
62 | WARNING | Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
62 | ERROR | Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
74 | WARNING | Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
74 | ERROR | Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
87 | WARNING | Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
87 | ERROR | Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
92 | WARNING | Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
92 | ERROR | Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
99 | WARNING | Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
99 | ERROR | Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
102 | WARNING | Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
102 | ERROR | Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
108 | WARNING | Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
108 | ERROR | Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
127 | WARNING | Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
127 | ERROR | Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
143 | WARNING | Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
143 | ERROR | Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
150 | WARNING | Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
150 | ERROR | Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
164 | WARNING | Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
164 | ERROR | Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
174 | WARNING | Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
174 | ERROR | Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
181 | WARNING | Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
181 | ERROR | Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
188 | WARNING | Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
188 | ERROR | Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
199 | WARNING | Possible useless method overriding detected
-------------------------------------------------------------------------------------------------------------------------

FILE: C:\wamp64\www\contribution\block_list_override\tests\src\Functional\BlockListOverrideTest.php
---------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------
92 | WARNING | Line exceeds 80 characters; contains 82 characters
---------------------------------------------------------------------------------------------------

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

sakthi_dev’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 22: phpcs_fix.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

zkhan.aamir’s picture

Hi,

MR #23 applied successfully.

Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules/block_list_override (1.0.x)
$ curl https://git.drupalcode.org/project/block_list_override/-/merge_requests/3.diff | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 22431    0 22431    0     0  52231      0 --:--:-- --:--:-- --:--:-- 52286
patching file README.txt
patching file block_list_override.install
patching file block_list_override.links.menu.yml
patching file config/install/block_list_override.settings.yml
patching file src/BlockListOverride.php
patching file src/Controller/DefaultController.php
patching file src/Controller/LayoutController.php
patching file src/Form/SettingsForm.php
patching file src/LayoutPluginAlter.php
patching file src/PluginAlter.php
patching file tests/src/Functional/BlockListOverrideTest.php

No errors remaining.

Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules
$ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,info,txt,md,css,js,yml block_list_override/

Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules
$

avpaderno’s picture

Issue summary: View changes
avpaderno’s picture

Status: Needs work » Needs review

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

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

roberttabigue’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new70.23 KB

Hi,

I have applied the latest MR!3 to the Block List Override module (1.0.x-dev) on my local Drupal setup, confirmed all PHPCS errors have been fixed and passes on the GitLab CI pipeline as well: https://git.drupalcode.org/issue/block_list_override-3157439/-/jobs/8228852.

I ran this command on the module:
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,info,txt,md,yml,css block_list_override

Please see the attached file for reference.

I'm moving this now to "RTBC".

Thank you!

heddn’s picture

Posted comment.

heddn’s picture

The removed test comments still are removed and there is a failure in https://git.drupalcode.org/issue/block_list_override-3157439/-/jobs/8259172

jerech’s picture

The word "blacklist" is not accepted by cspell despite having been added to the .txt dictionary.
You could change the word in the comments or in the text, but on line 27 of the log the following appears:

str_replace('block_blacklist', 'block_list_override', ...)

If block_blacklist refers to the actual name of an old module or an existing configuration key in the database, you cannot change that text string or the code will stop working (it will not find the old configuration to migrate).

heddn’s picture

I think we'll have to handle the blacklist issue in a follow-up. There's too many places that mention it. See https://git.drupalcode.org/search?search=blacklist&nav_source=navbar&pro...

And we'd need an update path for existing config.

heddn’s picture

There is still one minor issue with the spelling of the word, "existent".

jerech’s picture

Status: Reviewed & tested by the community » Needs review
roberttabigue’s picture

Status: Needs review » Reviewed & tested by the community

Hi,

I reviewed the CSPELL results in the GitLab CI pipeline and confirmed that the errors no longer exist.
Here is the link for reference:
CSPELL: https://git.drupalcode.org/issue/block_list_override-3157439/-/jobs/8422541

I'm moving this now to "RTBC".

Thank you!

  • heddn committed ff351a93 on 1.0.x authored by vishaljd
    feat: #3157439 Fix the issues reported by phpcs
    
    By: avpaderno
    By:...
heddn’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for the contributions.

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.