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
Comments
Comment #2
suresh prabhu parkala commentedComment #3
suresh prabhu parkala commentedPlease review the patch.
Comment #4
avpadernoCode 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.
Comment #5
kartiktandon commentedComment #6
ayush.khare commentedAdded reroll for #3 as it doesn't apply.
Comment #7
ayush.khare commentedRemoved commented out code as mentioned in #4.
Comment #8
kartiktandon commented@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.
Comment #9
avpadernoActually, the first point on Assigning ownership of issues / Assigning ownership of other project issues uses the following sentence. (Emphasis is mine.)
It doesn't take a week to change what reported in comment #4.
Comment #10
avpadernoThe 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.
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
phpcsreports, the issue summary should show which arguments are used forphpcsand 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.
Comment #12
vishaljd commentedComment #14
vishaljd commentedFixed coding standards using phpcs for Drupal and DrupalPractice standards.
Please check and merge pr.
Thanks
Comment #15
avpadernoSee my previous comment. The issue summary has not been updated.
Comment #16
avpadernoThe MR does not correctly change the files.
That is not what a documentation comment for a class should say. Adding of is not sufficient.
That change is not required by the coding standards.
There is no need to add a period to commented out code.
Comment #18
_pratik_Comment #19
avpadernoThe issue summary still needs to be updated.
Comment #20
avpadernoAs for the MR, it still uses Class of.
Comment #21
zkhan.aamir commentedIssue summary Updated.
Please review.
Comment #22
Shreyas gowda commentedresolved 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
---------------------------------------------------------------------------------------------------
Comment #24
sakthi_dev commentedComment #26
zkhan.aamir commentedHi,
MR #23 applied successfully.
No errors remaining.
Comment #27
avpadernoComment #28
avpadernoComment #31
roberttabigue commentedHi,
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_overridePlease see the attached file for reference.
I'm moving this now to "RTBC".
Thank you!
Comment #32
heddnPosted comment.
Comment #33
heddnThe removed test comments still are removed and there is a failure in https://git.drupalcode.org/issue/block_list_override-3157439/-/jobs/8259172
Comment #34
jerech commentedThe 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).
Comment #35
heddnI 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.
Comment #36
heddnThere is still one minor issue with the spelling of the word, "existent".
Comment #37
jerech commentedComment #38
roberttabigue commentedHi,
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!
Comment #40
heddnThanks for the contributions.