Problem/Motivation
PHPCS reported few errors and warnings mentioned in text document "PHPCS-report.txt"
Steps to reproduce
Execute the command: phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,js,info,txt,md,yml,twig integration_chargebee/
Proposed resolution
Fix all the errors and warnings
Remaining tasks
Patch Review.
Excluded "\Drupal calls should be avoided in classes, use dependency injection instead" error because it is already in progress with "https://www.drupal.org/project/integration_chargebee/issues/3360299"
Comments
Comment #2
avpadernoThe warning is the following.
Adding a comment before the
_permissionline does not change the required permission for those routes.Comment #5
rajan kumar commentedPlease review and merge code.
Comment #6
avpadernoCode lines are allowed to exceed 80 characters, if they are more readable. The existing code is more readable than the changed code.
The report does not say that
_access: 'TRUE'must be removed from that route definition, but that a comment explaining why_access: 'TRUE'is used must be added.The same holds true for this route definition.
ConfigFactory is misspelled.
The description is wrong.
Since that comment is change, also the method description must be changed: The description for a constructor must start with
Constructs a newfollowed by the class name (including its namespace), and end withobject.integration_chargebee is the module machine name and it does not describe this service's purpose.
Service is misspelled, since it must be spelled using lowercase letters.
Since that documentation comment is changed, also the property description must be changed, as property descriptions must not start with Drupal.
The request stack. is a better description.
The request stack. is a better description.
Since those comments have been changed, a definite article must be added to both the descriptions.
The description for a constructor must start with
Constructs a newfollowed by the class name (including its namespace), and end withobject.Comment #7
rajan kumar commentedComment #8
avpadernoThat does not explain why.
Only the first word in the description must be capitalized.
The class namespace is missing.
The indefinite article to use with user is the other one.
Since that documentation comment is changed, the definite article needs to be added to both those descriptions.
That is still repeating the class name. Adding spaces does not change that.
Since that documentation comment is changed, that description must be changed too: The config factory. is sufficient.
The verb must be declined to the third person singular.
Comment #9
rajan kumar commentedComment #10
avpadernoComment #11
shashank5563 commentedComment #12
shashank5563 commented@apaderno, For the permission issue, I will create another issue. Now, I merging this issue.
Comment #13
shashank5563 commented@apaderno, I am marking fixed this issue. I will create another issue for the permission.