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.

Issue fork sharethis-3084638

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

apaderno created an issue. See original summary.

avpaderno’s picture

Status: Active » Needs review
StatusFileSize
new70 bytes

The 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 [...]).

avpaderno’s picture

StatusFileSize
new1.85 KB

I am sorry: I uploaded the wrong patch.

avpaderno’s picture

I deleted the content of the wrong patch, to avoid confusion.

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

vladimiraus’s picture

Status: Needs review » Needs work

🍻 Thanks for the patch.
🏗️ Moving changes to MR!
🛑 More work required


FILE: /Users/tesboss/_PROJECTS/drupal/sharethis/sharethis.admin.inc
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 30 | WARNING | Unused variable $name.
----------------------------------------------------------------------


FILE: /Users/tesboss/_PROJECTS/drupal/sharethis/sharethis.module
---------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AND 1 WARNING AFFECTING 4 LINES
---------------------------------------------------------------------------------------------------------------------------------------------------
 228 | WARNING | [x] '@TODO: Switch from this function to just straight variable_get() calls.' should match the format '@todo Fix problem X here.'
 240 | ERROR   | [ ] The array declaration extends to column 174 (the limit is 80). The array content should be split up over multiple lines
 250 | ERROR   | [ ] The array declaration extends to column 100 (the limit is 80). The array content should be split up over multiple lines
 465 | ERROR   | [x] Use null coalesce operator instead of ternary operator.
---------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/tesboss/_PROJECTS/drupal/sharethis/plugins/content_types/sharethis/sharethis.inc
---------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------
 8 | WARNING | Unused variable $plugin.
---------------------------------------------------------------------------------------------


FILE: /Users/tesboss/_PROJECTS/drupal/sharethis/ShareThisForm.css
----------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
----------------------------------------------------------------------
  18 | ERROR | Style definitions must end with a semicolon
  40 | ERROR | Style definitions must end with a semicolon
  49 | ERROR | Style definitions must end with a semicolon
 149 | ERROR | Style definitions must end with a semicolon
----------------------------------------------------------------------


FILE: /Users/tesboss/_PROJECTS/drupal/sharethis/sharethis.install
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------
 46 | ERROR | The array declaration extends to column 84 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/tesboss/_PROJECTS/drupal/sharethis/stlib_picker.css
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 36 | ERROR | Style definitions must end with a semicolon
----------------------------------------------------------------------


FILE: /Users/tesboss/_PROJECTS/drupal/sharethis/views/sharethis_handler_field_link.inc
--------------------------------------------------------------------------------------
FOUND 2 ERRORS AND 1 WARNING AFFECTING 2 LINES
--------------------------------------------------------------------------------------
 11 | ERROR   | Class name must begin with a capital letter
 11 | ERROR   | Class name must use UpperCamel naming without underscores
 29 | WARNING | '@todo' should match the format '@todo Fix problem X here.'
--------------------------------------------------------------------------------------

Time: 574ms; Memory: 14MB


akram khan’s picture

Status: Needs work » Needs review
StatusFileSize
new7.62 KB

Added patch Fixed all remaining Coding standard issue

vladimiraus’s picture

Status: Needs review » Needs work

@Akram Khan your patch didn't apply successfully to 7 branch. Please use existing MR.

aditi saraf’s picture

Assigned: Unassigned » aditi saraf

Used 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

aditi saraf’s picture

Version: 7.x-2.x-dev » 8.x-2.x-dev
Status: Needs work » Needs review
StatusFileSize
new11.64 KB

Patch Applied , please review . There is some design issue is still there .

aditi saraf’s picture

Assigned: aditi saraf » Unassigned
avpaderno’s picture

Version: 8.x-2.x-dev » 7.x-2.x-dev
Status: Needs review » Needs work

This issue is for the 7.x-2.x branch.

vladimiraus’s picture

Thanks @apaderno.
Please use corresponding MR @AditiVB.

vladimiraus’s picture

vladimiraus’s picture

Status: Needs work » Needs review

All fixed except two things that are there by design

FILE: /Users/tesboss/_PROJECTS/drupal/sharethis/plugins/content_types/sharethis/sharethis.inc
---------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------
 8 | WARNING | Unused variable $plugin.
---------------------------------------------------------------------------------------------


FILE: /Users/tesboss/_PROJECTS/drupal/sharethis/views/sharethis_handler_field_link.inc
--------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
--------------------------------------------------------------------------------------
 11 | ERROR | Class name must begin with a capital letter
 11 | ERROR | Class name must use UpperCamel naming without underscores
--------------------------------------------------------------------------------------

Time: 585ms; Memory: 12MB

avpaderno’s picture

Status: Needs review » Reviewed & tested by the community

Yes, classes used by Views do not use UpperCamel naming.
I did not find any problem in the MR created by VladimirAus.

  • VladimirAus committed 727c15b4 on 7.x-2.x
    Issue #3084638 by VladimirAus, apaderno, AditiVB, Akram Khan: Fix coding...
vladimiraus’s picture

Status: Reviewed & tested by the community » Fixed

Fixed and committed! Thank you 🍻

  • VladimirAus committed 833e2c57 on 7.x-2.x authored by AditiVB
    Issue #3084638 by VladimirAus, apaderno, AditiVB, Akram Khan: Fix coding...

  • VladimirAus committed dabd691b on 7.x-2.x authored by apaderno
    Issue #3084638 by apaderno, VladimirAus: Fix coding standard related...

Status: Fixed » Closed (fixed)

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

weseze’s picture

The 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?

avpaderno’s picture

?: is supported starting from PHP 5.3.0; ?? is supported starting from PHP 7.0.

weseze’s picture

OK, 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.

avpaderno’s picture

?? 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.