Problem/Motivation

FILE: ...-contrib/modules/ctools/src/Form/ManageResolverRelationships.php
----------------------------------------------------------------------
FOUND 43 ERRORS AND 3 WARNINGS AFFECTING 42 LINES
----------------------------------------------------------------------
  15 | ERROR   | [x] Missing class doc comment
  17 | ERROR   | [ ] Missing short description in doc comment
  20 | ERROR   | [ ] Class property $machine_name should use
     |         |     lowerCamel naming without underscores
  27 | ERROR   | [ ] Class property $property_types should use
     |         |     lowerCamel naming without underscores
  79 | ERROR   | [x] Short array syntax must be used to define arrays
  84 | ERROR   | [x] Short array syntax must be used to define arrays
  84 | ERROR   | [ ] If the line declaring an array spans longer than
     |         |     80 characters, each element should be broken
     |         |     into its own line
  86 | WARNING | [x] A comma should follow the last multiline array
     |         |     item. Found: )
 104 | WARNING | [x] A comma should follow the last multiline array
     |         |     item. Found: ]
 120 | ERROR   | [x] Missing function doc comment
 135 | ERROR   | [x] Short array syntax must be used to define arrays
 139 | ERROR   | [x] Missing function doc comment
 145 | ERROR   | [ ] Missing short description in doc comment
 146 | ERROR   | [ ] Missing parameter comment
 146 | ERROR   | [ ] Missing parameter type
 148 | ERROR   | [ ] Description for the @return value is missing
 151 | ERROR   | [x] Short array syntax must be used to define arrays
 154 | ERROR   | [x] Short array syntax must be used to define arrays
 158 | ERROR   | [x] Short array syntax must be used to define arrays
 162 | ERROR   | [ ] Key specified for array entry; first entry has
     |         |     no key
 170 | ERROR   | [ ] Missing short description in doc comment
 171 | ERROR   | [ ] Missing parameter comment
 172 | ERROR   | [ ] Missing parameter comment
 173 | ERROR   | [ ] Missing parameter comment
 174 | ERROR   | [ ] Missing parameter comment
 176 | ERROR   | [ ] Description for the @return value is missing
 178 | ERROR   | [ ] Type hint "array" missing for $cached_values
 178 | ERROR   | [x] Short array syntax must be used to define arrays
 179 | WARNING | [ ] Line exceeds 80 characters; contains 95
     |         |     characters
 183 | ERROR   | [x] Short array syntax must be used to define arrays
 187 | ERROR   | [x] Short array syntax must be used to define arrays
 196 | ERROR   | [x] Short array syntax must be used to define arrays
 200 | ERROR   | [x] Short array syntax must be used to define arrays
 201 | ERROR   | [x] Short array syntax must be used to define arrays
 217 | ERROR   | [ ] Description for the @return value is missing
 224 | ERROR   | [ ] Description for the @return value is missing
 231 | ERROR   | [ ] Description for the @return value is missing
 235 | ERROR   | [ ] Missing short description in doc comment
 236 | ERROR   | [ ] Missing parameter comment
 236 | ERROR   | [ ] Missing parameter type
 238 | ERROR   | [ ] Description for the @return value is missing
 242 | ERROR   | [ ] Missing short description in doc comment
 243 | ERROR   | [ ] Missing parameter comment
 244 | ERROR   | [ ] Missing parameter comment
 245 | ERROR   | [ ] Missing parameter comment
 247 | ERROR   | [ ] Description for the @return value is missing
----------------------------------------------------------------------
PHPCBF CAN FIX THE 17 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
CommentFileSizeAuthor
#14 ctools-ctools-Standards_on_Form_ManageResolverRelationships-3033371-14-D8.patch7.83 KBthalles
#12 ctools-ctools-Standards_on_Form_ManageResolverRelationships-3033371-12-D8.patch9.36 KBthalles
#10 ctools-ctools-Standards_on_Form_ManageResolverRelationships-3033371-10-D8.patch11.11 KBthalles
#9 ctools-ctools-Standards_on_Form_ManageResolverRelationships-3033371-9-D8.patch7.98 KBthalles
#7 ctools-ctools-Standards_on_Form_ManageResolverRelationships-3033371-7-D8.patch8.02 KBthalles
#5 ctools-ctools-Standards_on_Form_ManageResolverRelationships-3033371-5-D8.patch9.61 KBthalles
#2 ctools-Standards_on_Form_ManageResolverRelationships-3033371-2-D8.patch9.64 KBthalles

Comments

thalles created an issue. See original summary.

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
StatusFileSize
new ctools-Standards_on_Form_ManageResolverRelationships-3033371-2-D8.patch9.64 KB

Follow the patch!

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented 
+++ b/src/Form/ManageResolverRelationships.php
@@ -12,19 +12,24 @@ use Drupal\Core\Url;
-  protected $machine_name;
+  protected $machineName;

These changes could break anything extending from this abstract class.

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented

We can use both and warn that the former went into depreciation.

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
StatusFileSize
new ctools-ctools-Standards_on_Form_ManageResolverRelationships-3033371-5-D8.patch9.61 KB

Follow the patch!

Status: Needs review » Needs work

The last submitted patch, 5: ctools-ctools-Standards_on_Form_ManageResolverRelationships-3033371-5-D8.patch, failed testing. View results

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Status: Needs work » Needs review
StatusFileSize
new ctools-ctools-Standards_on_Form_ManageResolverRelationships-3033371-7-D8.patch8.02 KB

Follow new!

Status: Needs review » Needs work

The last submitted patch, 7: ctools-ctools-Standards_on_Form_ManageResolverRelationships-3033371-7-D8.patch, failed testing. View results

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Status: Needs work » Needs review
StatusFileSize
new ctools-ctools-Standards_on_Form_ManageResolverRelationships-3033371-9-D8.patch7.98 KB

Follow the patch!

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
StatusFileSize
new ctools-ctools-Standards_on_Form_ManageResolverRelationships-3033371-10-D8.patch11.11 KB

This patch triggers the message that non-camel variables are depreciated.

andrey.troeglazov’s picture

andrey.troeglazov commented

This is a bad solution from my point of view, I think adding @deprecated message in doc will be enough.

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
StatusFileSize
new ctools-ctools-Standards_on_Form_ManageResolverRelationships-3033371-12-D8.patch9.36 KB

Follow a patch!

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Needs work

This looks great but I have a couple of concerns that need to be addressed:

  1. +++ b/src/Form/ManageResolverRelationships.php
@@ -12,9 +12,14 @@ use Drupal\Core\Url;
   protected $machine_name;

@@ -75,16 +80,18 @@ abstract class ManageResolverRelationships extends FormBase {
-    $this->machine_name = $cached_values['id'];
...
+    $this->machineName = $cached_values['id'];

    I don't think machine_name to machineName is ideal to change as child classes may be using that protected property. 

  2. +++ b/src/Form/ManageResolverRelationships.php
@@ -143,68 +164,80 @@ abstract class ManageResolverRelationships extends FormBase {
+        '0' => $row,

    why is this changing to a string key?

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Status: Needs work » Needs review
StatusFileSize
new ctools-ctools-Standards_on_Form_ManageResolverRelationships-3033371-14-D8.patch7.83 KB

Hi @joelpittet!
All right?
Follow a new patch with the proposed changes.

About key:
I added because phpcs asked because it has a key in one value and in others not.

Thanks!

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Fixed

Thanks for doing that, strange that PHPCS would mention mixed indexes. I've committed this to the latest dev.

  • joelpittet committed b2ae014 on 8.x-3.x authored by thalles 
    Issue #3033371 by thalles: Standards on Form/ManageResolverRelationships...

Status: Fixed » Closed (fixed)

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