Running phpcs ./ --standard=Drupal,DrupalPractice outputs the following errors.
FILE: /home/kiamlaluno/patches/sharethis/sharethis.admin.inc ------------------------------------------------------------------------- FOUND 2 ERRORS AND 1 WARNING AFFECTING 2 LINES ------------------------------------------------------------------------- 30 | WARNING | [ ] Unused variable $name. 73 | ERROR | [x] Concat operator must be surrounded by a single space 73 | ERROR | [x] Concat operator must be surrounded by a single space ------------------------------------------------------------------------- PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------- FILE: /home/kiamlaluno/patches/sharethis/sharethis.module --------------------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE --------------------------------------------------------------------------------------------- 226 | ERROR | [x] Hook implementations must be documented with "Implements hook_example()." --------------------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY --------------------------------------------------------------------------------------------- FILE: /home/kiamlaluno/patches/sharethis/sharethis.install ---------------------------------------------------------------------------------------- FOUND 2 ERRORS AFFECTING 2 LINES ---------------------------------------------------------------------------------------- 83 | ERROR | [x] There must be exactly one blank line before the tags in a doc comment 96 | ERROR | [x] There must be exactly one blank line before the tags in a doc comment ---------------------------------------------------------------------------------------- PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY ----------------------------------------------------------------------------------------
They should be fixed, so the automatic tests running when a patch is submitted run without coding standard errors.
| Comment | File | Size | Author |
|---|
Issue fork sharethis-3084638
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:
- 3084638-fix-coding-standard
changes, plain diff MR !5
- 7.x-2.x
changes, plain diff MR !7
Comments
Comment #2
avpadernoThe error about
sharethis_get_options_array()is a false positive, since that function isn't a hook implementation. I corrected the description of two functions, since they should start with a verb on the third person singular (Creates [...] instead of This function is for creating [...]).Comment #3
avpadernoI am sorry: I uploaded the wrong patch.
Comment #4
avpadernoI deleted the content of the wrong patch, to avoid confusion.
Comment #7
vladimiraus🍻 Thanks for the patch.
🏗️ Moving changes to MR!
🛑 More work required
Comment #8
akram khanAdded patch Fixed all remaining Coding standard issue
Comment #9
vladimiraus@Akram Khan your patch didn't apply successfully to 7 branch. Please use existing MR.
Comment #11
aditi saraf commentedUsed phpcs command for checking coding standard issue , And got huge no of issue as mentioned below . I will work on it and fix the issue .
FILE: /var/www/html/drupal9/modules/contrib/sharethis/src/SharethisManager.php
----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------------------------------------------------
7 | WARNING | [x] Unused use statement
81 | WARNING | [ ] NodeType::loadMultiple calls should be avoided in classes, use dependency injection instead
----------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------
FILE: /var/www/html/drupal9/modules/contrib/sharethis/src/Form/SharethisConfigurationForm.php
----------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------
117 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
----------------------------------------------------------------------------------------------
FILE: /var/www/html/drupal9/modules/contrib/sharethis/tests/src/Functional/SharethisConfigFormTest.php
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 4 WARNINGS AFFECTING 5 LINES
-----------------------------------------------------------------------------------------------------------------------------------------
48 | ERROR | The array declaration extends to column 91 (the limit is 80). The array content should be split up over multiple lines
59 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
60 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
84 | 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
-----------------------------------------------------------------------------------------------------------------------------------------
FILE: /var/www/html/drupal9/modules/contrib/sharethis/tests/src/Functional/Views/SharethisViewsPluginTest.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------
53 | ERROR | The array declaration extends to column 116 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------------------------------------------------------------
FILE: /var/www/html/drupal9/modules/contrib/sharethis/tests/src/Functional/SharethisBlockTest.php
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
-----------------------------------------------------------------------------------------------------------------------------------------
47 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
48 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
60 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
61 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
-----------------------------------------------------------------------------------------------------------------------------------------
FILE: /var/www/html/drupal9/modules/contrib/sharethis/tests/src/Functional/ShareThisTest.php
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------------------------------
41 | ERROR | The array declaration extends to column 88 (the limit is 80). The array content should be split up over multiple lines
54 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
-----------------------------------------------------------------------------------------------------------------------------------------
FILE: /var/www/html/drupal9/modules/contrib/sharethis/css/sharethis.form.css
------------------------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
------------------------------------------------------------------------------------------------
13 | ERROR | [ ] Style definitions must end with a semicolon
23 | ERROR | [x] CSS colours must be defined in lowercase; expected #056d2d but found #056D2D
35 | ERROR | [ ] Style definitions must end with a semicolon
44 | ERROR | [ ] Style definitions must end with a semicolon
144 | ERROR | [ ] Style definitions must end with a semicolon
------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------
FILE: /var/www/html/drupal9/modules/contrib/sharethis/css/stlib_picker.css
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
32 | ERROR | Style definitions must end with a semicolon
--------------------------------------------------------------------------
Time: 218ms; Memory: 12MB
Comment #12
aditi saraf commentedPatch Applied , please review . There is some design issue is still there .
Comment #13
aditi saraf commentedComment #14
avpadernoThis issue is for the 7.x-2.x branch.
Comment #16
vladimirausThanks @apaderno.
Please use corresponding MR @AditiVB.
Comment #17
vladimirausComment #18
vladimirausAll fixed except two things that are there by design
Comment #19
avpadernoYes, classes used by Views do not use UpperCamel naming.
I did not find any problem in the MR created by VladimirAus.
Comment #21
vladimirausFixed and committed! Thank you 🍻
Comment #25
weseze commentedThe changes here broke compatibility with php5.x since the shorthand ternary operators (?? and ?:) are not supported on PHP5.x, only 7.x and higher.
Don't know if it is worth "fixing" though?
Comment #26
avpaderno?:is supported starting from PHP 5.3.0;??is supported starting from PHP 7.0.Comment #27
weseze commentedOK, it is only ?? then.
They ware added in commits on this ticket:
https://git.drupalcode.org/project/sharethis/-/commit/833e2c574fb1b2a82b...
https://git.drupalcode.org/project/sharethis/-/commit/727c15b4f173f8d63e...
Again, not sure if it is worth fixing...
I have just locked down this module to the previous version and won't bother updating further within the Drupal7 scope. D7 is basically EOL anyway.
Comment #28
avpaderno??cannot be used with Drupal 7, since its minimum PHP version is lower than the PHP version that supports that operator. The code using it should be changed.When I reviewed the MR, I forgot it was for a Drupal 7 branch.