Problem/Motivation

Getting following error/warnings

FILE: /app/modules/contrib/commerce_license/README.md
----------------------------------------------------------------------
FOUND 0 ERRORS AND 15 WARNINGS AFFECTING 15 LINES
----------------------------------------------------------------------
25 | WARNING | Line exceeds 80 characters; contains 114 characters
49 | WARNING | Line exceeds 80 characters; contains 82 characters
50 | WARNING | Line exceeds 80 characters; contains 96 characters
52 | WARNING | Line exceeds 80 characters; contains 89 characters
53 | WARNING | Line exceeds 80 characters; contains 87 characters
55 | WARNING | Line exceeds 80 characters; contains 82 characters
56 | WARNING | Line exceeds 80 characters; contains 100 characters
69 | WARNING | Line exceeds 80 characters; contains 82 characters
70 | WARNING | Line exceeds 80 characters; contains 96 characters
72 | WARNING | Line exceeds 80 characters; contains 89 characters
73 | WARNING | Line exceeds 80 characters; contains 87 characters
75 | WARNING | Line exceeds 80 characters; contains 82 characters
76 | WARNING | Line exceeds 80 characters; contains 100 characters
82 | WARNING | Line exceeds 80 characters; contains 88 characters
88 | WARNING | Line exceeds 80 characters; contains 81 characters
----------------------------------------------------------------------

FILE: /app/modules/contrib/commerce_license/commerce_license.page.inc
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
24 | WARNING | Unused variable $commerce_license.
----------------------------------------------------------------------

FILE: /app/modules/contrib/commerce_license/src/EventSubscriber/LicenseRenewalCartEventSubscriber.php
-----------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------
123 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-----------------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/commerce_license/src/Form/LicenseCheckoutForm.php
---------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
---------------------------------------------------------------------------------------------
47 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
48 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
---------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/commerce_license/src/LicenseAvailabilityCheckerExistingRights.php
----------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------
142 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
----------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/commerce_license/src/LicenseOrderProcessorMultiples.php
----------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------
43 | WARNING | There must be no blank line following an inline comment
----------------------------------------------------------------------------------

FILE: /app/modules/contrib/commerce_license/src/Plugin/Commerce/LicenseType/Role.php
-------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------
131 | WARNING | UserRole::loadMultiple calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------------------------------------

FILE: /app/modules/contrib/commerce_license/tests/src/Kernel/CommerceAvailabilityExistingRightsTest.php
-------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------
59 | WARNING | Unused variable $order_type.
-------------------------------------------------------------------------------------------------------

Time: 4.58 secs; Memory: 14MB

Steps to reproduce

Run following command

phpcs --standard="Drupal,DrupalPractice" --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml modules/contrib/commerce_license/

Proposed resolution

Above error/warnings need to be fixed

CommentFileSizeAuthor
#11 coding-standards-3330264-11.patch9.74 KBtomtech
#9 interdiff_5-9.txt5.99 KBhardikpandya
#9 3330264-9.patch11.02 KBhardikpandya
#5 interdiff_2-5.txt445 bytessamitk
#5 3330264-5.patch17.01 KBsamitk
#2 3330264-2.patch17.03 KBsamitk

Issue fork commerce_license-3330264

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

samit.310@gmail.com created an issue. See original summary.

samitk’s picture

samitk commented
Assigned: samitk » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3330264-2.patch17.03 KB

Error/warnings are fixed.

Status: Needs review » Needs work

The last submitted patch, 2: 3330264-2.patch, failed testing. View results

Ameer Khan made their first commit to this issue’s fork.

samitk’s picture

samitk commented
Status: Needs work » Needs review
StatusFileSize
new 3330264-5.patch17.01 KB
new interdiff_2-5.txt445 bytes

Failed test cases issue fixed.

interdiff with #2

atul_ghate’s picture

atul_ghate commented
Assigned: Unassigned » atul_ghate

I will review this patch.

atul_ghate’s picture

atul_ghate commented
Assigned: atul_ghate » Unassigned
Status: Needs review » Reviewed & tested by the community

I have reviewed and applied the patch cleanly it resolved all phpcs issues, moving to RTBC.

var/www/html/d9test/web $ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml --encoding=utf-8 -p modules/contrib/commerce_license/ --ignore=node_modules
............................................................ 60 / 92 (65%)
................................ 92 / 92 (100%)

Time: 2.1 secs; Memory: 14MB

rszrama’s picture

rszrama commented
Category: Bug report » Task
Status: Reviewed & tested by the community » Needs work
Issue tags: -, -

It's perfectly fine for markdown file lines to exceed 80 characters.

hardikpandya’s picture

hardikpandya commented
Status: Needs work » Needs review
StatusFileSize
new 3330264-9.patch11.02 KB
new interdiff_5-9.txt5.99 KB

Reverted md file changes.

Status: Needs review » Needs work

The last submitted patch, 9: 3330264-9.patch, failed testing. View results

tomtech’s picture

tomtech commented
Assigned: Unassigned » tomtech
Status: Needs work » Needs review
StatusFileSize
new coding-standards-3330264-11.patch9.74 KB

Thanks for these contributions.

Several of the changes include unnecessary properties being added, e.g. adding $time as a property on the LicenseCheckoutForm.php class is unnecessary, as its parent class, ContentEntityForm.php, already has a $time property.

Attached is a revised patch that should address all coding standards issues.

  • TomTech authored 82cdda67 on 8.x-2.x 
    Issue #3330264 by samit.310@gmail.com, hardikpandya, TomTech, rszrama:...
tomtech’s picture

tomtech commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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