Problem/Motivation

FILE: ~/projects/drupal-d10/modules/contrib/viewfield/viewfield.module
-------------------------------------------------------------------------------------------------
FOUND 9 ERRORS AND 3 WARNINGS AFFECTING 9 LINES
-------------------------------------------------------------------------------------------------
  20 | ERROR   | [ ] The array declaration extends to column 1052 (the limit is 80). The array content should be split up over multiple lines
 100 | ERROR   | [x] Use null coalesce operator instead of ternary operator.
 105 | ERROR   | [x] Use null coalesce operator instead of ternary operator.
 107 | ERROR   | [x] Use null coalesce operator instead of ternary operator.
 110 | ERROR   | [ ] Comment indentation error, expected only 1 spaces
 111 | ERROR   | [ ] Comment indentation error, expected only 2 spaces
 114 | WARNING | [ ] Line exceeds 80 characters; contains 124 characters
 114 | ERROR   | [ ] Comment indentation error, expected only 1 spaces
 115 | WARNING | [ ] Line exceeds 80 characters; contains 128 characters
 115 | ERROR   | [ ] Comment indentation error, expected only 2 spaces
 136 | WARNING | [ ] Line exceeds 80 characters; contains 188 characters
 136 | ERROR   | [ ] Comment indentation error, expected only 1 spaces
-------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------


FILE: ~/projects/drupal-d10/modules/contrib/viewfield/tests/src/FunctionalJavascript/ViewfieldFormatterTest.php
---------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 5 WARNINGS AFFECTING 5 LINES
---------------------------------------------------------------------------------------------
  46 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  85 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 129 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 176 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 222 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
---------------------------------------------------------------------------------------------


FILE: ~/projects/drupal-d10/modules/contrib/viewfield/tests/src/FunctionalJavascript/ViewfieldFunctionalTestBase.php
------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
------------------------------------------------------------------------------------------------
  32 | ERROR | [ ] Missing @var tag in member variable comment
  64 | ERROR | [ ] The array declaration extends to column 100 (the limit is 80). The array content should be split up over multiple lines
 165 | ERROR | [x] Expected 1 blank line after function; 2 found
 168 | ERROR | [x] The closing brace for the class must have an empty line before it
------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------


FILE: ~/projects/drupal-d10/modules/contrib/viewfield/tests/src/FunctionalJavascript/ViewfieldWidgetTest.php
------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 2 WARNINGS AFFECTING 3 LINES
------------------------------------------------------------------------------------------------
 26 | WARNING | [ ] Unused variable $page.
 50 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 58 | ERROR   | [x] Expected 1 newline at end of file; 0 found
------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------


FILE: ~/projects/drupal-d10/modules/contrib/viewfield/tests/src/Kernel/ViewfieldKernelTest.php
------------------------------------------------------------------------------------------------------------------------------
FOUND 6 ERRORS AND 18 WARNINGS AFFECTING 13 LINES
------------------------------------------------------------------------------------------------------------------------------
 28 | ERROR   | [ ] Missing @var tag in member variable comment
 31 | ERROR   | [x] Missing function doc comment
 32 | WARNING | [x] 'TODO: Change the autogenerated stub' should match the format '@todo Fix problem X here.'
 32 | WARNING | [ ] There must be no blank line following an inline comment
 32 | ERROR   | [x] Comments may not appear after statements
 32 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 59 | ERROR   | [x] Missing function doc comment
 60 | WARNING | [x] There must be no blank line following an inline comment
 60 | WARNING | [ ] There must be no blank line following an inline comment
 62 | WARNING | [x] There must be no blank line following an inline comment
 62 | WARNING | [ ] There must be no blank line following an inline comment
 64 | WARNING | [x] There must be no blank line following an inline comment
 64 | WARNING | [ ] There must be no blank line following an inline comment
 66 | WARNING | [x] There must be no blank line following an inline comment
 66 | WARNING | [ ] There must be no blank line following an inline comment
 68 | WARNING | [x] There must be no blank line following an inline comment
 68 | WARNING | [ ] There must be no blank line following an inline comment
 70 | WARNING | [x] There must be no blank line following an inline comment
 70 | WARNING | [ ] There must be no blank line following an inline comment
 72 | WARNING | [x] There must be no blank line following an inline comment
 72 | WARNING | [ ] There must be no blank line following an inline comment
 74 | WARNING | [x] There must be no blank line following an inline comment
 74 | WARNING | [ ] There must be no blank line following an inline comment
 78 | ERROR   | [x] Missing function doc comment
------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 14 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------


FILE: ~/projects/drupal-d10/modules/contrib/viewfield/viewfield.libraries.yml
------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------
 9 | ERROR | [x] Expected 1 newline at end of file; 0 found
------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------


FILE: ~/projects/drupal-d10/modules/contrib/viewfield/README.txt
-----------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------
 54 | WARNING | Line exceeds 80 characters; contains 81 characters
-----------------------------------------------------------------------------------------


FILE: ~/projects/drupal-d10/modules/contrib/viewfield/viewfield.install
------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------
 128 | ERROR | [x] Expected 1 newline at end of file; 0 found
------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------


FILE: ~/projects/drupal-d10/modules/contrib/viewfield/src/ViewfieldServiceProvider.php
---------------------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
---------------------------------------------------------------------------------------------------------------
 11 | ERROR | [x] Doc comment short description must end with a full stop
 22 | ERROR | [x] Short array syntax must be used to define arrays
 26 | ERROR | [x] Short array syntax must be used to define arrays
 31 | ERROR | [x] Expected 1 newline at end of file; 0 found
---------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------


FILE: ~/projects/drupal-d10/modules/contrib/viewfield/src/Plugin/Field/FieldWidget/ViewfieldWidgetSelect.php
----------------------------------------------------------------------------------------
FOUND 8 ERRORS AND 3 WARNINGS AFFECTING 8 LINES
----------------------------------------------------------------------------------------
  92 | ERROR   | [x] Expected 1 space after "="; 2 found
 111 | ERROR   | [x] TRUE, FALSE and NULL must be uppercase; expected "FALSE" but found "false"
 115 | ERROR   | [x] Concat operator must be surrounded by a single space
 116 | ERROR   | [x] Concat operator must be surrounded by a single space
 163 | ERROR   | [x] Whitespace found at end of line
 244 | ERROR   | [x] Missing function doc comment
 245 | WARNING | [ ] Line exceeds 80 characters; contains 108 characters
 245 | WARNING | [x] 'TODO: Change the autogenerated stub' should match the format '@todo Fix problem X here.'
 245 | ERROR   | [x] Comments may not appear after statements
 245 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 246 | WARNING | [ ] Unused variable $key.
----------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 9 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------


FILE: ~/projects/drupal-d10/modules/contrib/viewfield/src/Plugin/Field/FieldType/ViewfieldItem.php
-----------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 2 WARNINGS AFFECTING 3 LINES
-----------------------------------------------------------------------------------------------
   5 | WARNING | [x] Unused use statement
   8 | WARNING | [x] Unused use statement
 138 | ERROR   | [ ] The array declaration extends to column 84 (the limit is 80). The array content should be split up over multiple lines
-----------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------


FILE: ~/projects/drupal-d10/modules/contrib/viewfield/src/Plugin/Field/FieldFormatter/ViewfieldFormatterDefault.php
-----------------------------------------------------------------------------------------------
FOUND 2 ERRORS AND 1 WARNING AFFECTING 3 LINES
-----------------------------------------------------------------------------------------------
 154 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 162 | ERROR   | [x] Expected 1 space after IF keyword; 0 found
 249 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
-----------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------

Time: 251ms; Memory: 12MB

Steps to reproduce

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml viewfield

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#20 3127062-MR-6-Before-Apply.png2.43 MBrassoni
#20 3127062-MR-6-After-Apply.png683.53 KBrassoni
#16 3127062-16.patch9.08 KBmrinalini9
#14 3127062-8.patch9.06 KBmaithri shetty
#11 interdiff_8_10.txt10.82 KBrassoni
#11 3127062-10.patch10.39 KBrassoni
#9 3127062-8.patch9.06 KBmaithri shetty
#7 3127062-4.patch20.65 KBrassoni
#5 interdiff_4_5.txt19.03 KBrassoni
#5 3127062-3.patch18.6 KBrassoni
#4 3127062-2.patch9.06 KBmaithri shetty
#2 3127062-2.patch9.06 KBmaithri shetty

Issue fork viewfield-3127062

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

Maithri Shetty created an issue. See original summary.

maithri shetty’s picture

maithri shetty commented
Status: Active » Needs review
StatusFileSize
new 3127062-2.patch9.06 KB

Status: Needs review » Needs work

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

maithri shetty’s picture

maithri shetty commented
StatusFileSize
new 3127062-2.patch9.06 KB
rassoni’s picture

rassoni commented
Status: Needs work » Needs review
StatusFileSize
new 3127062-3.patch18.6 KB
new interdiff_4_5.txt19.03 KB

Status: Needs review » Needs work

The last submitted patch, 5: 3127062-3.patch, failed testing. View results

rassoni’s picture

rassoni commented
Status: Needs work » Needs review
StatusFileSize
new 3127062-4.patch20.65 KB

Fixed failed issue.

Status: Needs review » Needs work

The last submitted patch, 7: 3127062-4.patch, failed testing. View results

maithri shetty’s picture

maithri shetty commented
Status: Needs work » Needs review
StatusFileSize
new 3127062-8.patch9.06 KB

Status: Needs review » Needs work

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

rassoni’s picture

rassoni commented
StatusFileSize
new 3127062-10.patch10.39 KB
new interdiff_8_10.txt10.82 KB
rassoni’s picture

rassoni commented
Status: Needs work » Needs review
jerdavis’s picture

jerdavis
Location Minneapolis, MN
commented
Status: Needs review » Needs work

The patch is failing to apply

maithri shetty’s picture

maithri shetty commented
Status: Needs work » Needs review
StatusFileSize
new 3127062-8.patch9.06 KB
indrapatil’s picture

indrapatil commented
Status: Needs review » Needs work

Patch failed

dell@indra-latitude-5480:/var/www/html/drupal-8/viewfield(8.x-3.x)$ git apply 3127062-8.patch
error: patch failed: tests/src/Kernel/ViewfieldKernelTest.php:23
error: tests/src/Kernel/ViewfieldKernelTest.php: patch does not apply

So moving to needs work.

mrinalini9’s picture

mrinalini9 commented
Status: Needs work » Needs review
StatusFileSize
new 3127062-16.patch9.08 KB

Rerolled patch #14, please review it.

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

VladimirAus opened merge request !6

vladimiraus’s picture

vladimiraus
Primary language English
Location Brisbane 🇦🇺
commented

Rerolled as MR and fixed everything except

FILE: /Users/tesboss/_PROJECTS/drupal/viewfield/src/Plugin/Field/FieldFormatter/ViewfieldFormatterDefault.php
-------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------
 249 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------------------------------------
rassoni’s picture

rassoni commented
StatusFileSize
new 3127062-MR-6-After-Apply.png683.53 KB
new 3127062-MR-6-Before-Apply.png2.43 MB

MR 6 review and tested locally and PHPCS issues are resolved. Please check the screenshot for reference.

rassoni’s picture

rassoni commented
Status: Needs review » Reviewed & tested by the community
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Phpcs coding standard » Fix the issues reported by phpcs
Status: Reviewed & tested by the community » Needs work
Issue tags: +Coding standards, +Needs issue summary update

The issue summary should always describe what the issue is trying to fix and, in the case, of coding standards issues, show which command has been used, which arguments have been used, and which report that command shown.

rassoni’s picture

rassoni commented
Issue summary: View changes
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Needs review
Issue tags: -Needs issue summary update
vladimiraus’s picture

vladimiraus
Primary language English
Location Brisbane 🇦🇺
commented
Status: Needs review » Reviewed & tested by the community
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Priority: Normal » Minor
Status: Reviewed & tested by the community » Needs review 
+  /**
+   * The Token service.
+   *
+   * @var \Drupal\Core\Utility\Token

Only the first word in the description must be capitalized.

+  /**
+   * Constructs a StringFormatter instance.

The class is not correct; the class is ViewfieldFormatterDefault.
The description for a constructor must start with Constructs a new followed by the class name (including its namespace), and end with object.

-        // Set arguments if they exist
+        // Set arguments if they exist.
         if (!empty($arguments)) {
           $view->setArguments($arguments);
         }

That comment can be removed, since it describes something already clear from the code.

+ // @todo Change the autogenerated stub

The period at the end is missing.

 /**
- * Service Provider for Viewfield
+ * Service Provider for Viewfield.
  */
 class ViewfieldServiceProvider extends ServiceProviderBase {

That description does not say what the class does.
Only the first word must be capitalized.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work
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
Assigned: nitin_lama » Unassigned

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

sakthi_dev’s picture

sakthi_dev commented
Status: Needs work » Needs review

Updated the MR with the changes mentioned in #26. Please review.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work 
-        // Set arguments if they exist
+        // Set arguments if they exist.
         if (!empty($arguments)) {
           $view->setArguments($arguments);
         }

That comment can be removed, since it describes something already clear from the code.

- * Service Provider for Viewfield
+ * Sets definition for searializer and tag for normalizer.

There is a typo in that description, which does not make sense.

+  /**
+   * Test view field item default value.
+   */

The verb must use the third singular person.
The description should be rewritten: Four attributive nouns are too much.

-  //  if (!isset($default_attributes)) {
-  //    $default_attributes = new Attribute();
-  //  }
+  // if (!isset($default_attributes)) {
+  // $default_attributes = new Attribute();
+  // }

Even if commented out, that is still code. The existing line are correctly indented; the changed code is not correctly indented.

-    //    $variables['items'][$delta]['attributes'] = !empty($element['#items'][$delta]->_attributes) ? new Attribute($element['#items'][$delta]->_attributes) : clone($default_attributes);
+    // $variables['items'][$delta]['attributes'] =
+    // !empty($element['#items'][$delta]->_attributes) ?
+    // new Attribute($element['#items'][$delta]->_attributes) :
+    // clone($default_attributes);

Code lines can exceed 80 characters, if they are more readable. That is still valid for commented out code.

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
Assigned: nitin_lama » Unassigned