Problem/Motivation

Fix the issues reported by PHPCS for Drupal and DrupalPractice standards in 2.0.x branch

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 smart_trim/

Proposed resolution

Fix all the issues mentioned below:

FILE: ...www/html/contribution/drupal8/web/modules/smart_trim-3351447/CHANGELOG.txt
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 16 WARNINGS AFFECTING 16 LINES
--------------------------------------------------------------------------------
3 | WARNING | Line exceeds 80 characters; contains 81 characters
4 | WARNING | Line exceeds 80 characters; contains 87 characters
5 | WARNING | Line exceeds 80 characters; contains 82 characters
10 | WARNING | Line exceeds 80 characters; contains 89 characters
12 | WARNING | Line exceeds 80 characters; contains 141 characters
13 | WARNING | Line exceeds 80 characters; contains 97 characters
14 | WARNING | Line exceeds 80 characters; contains 88 characters
15 | WARNING | Line exceeds 80 characters; contains 95 characters
18 | WARNING | Line exceeds 80 characters; contains 89 characters
19 | WARNING | Line exceeds 80 characters; contains 91 characters
25 | WARNING | Line exceeds 80 characters; contains 134 characters
27 | WARNING | Line exceeds 80 characters; contains 87 characters
28 | WARNING | Line exceeds 80 characters; contains 97 characters
32 | WARNING | Line exceeds 80 characters; contains 118 characters
33 | WARNING | Line exceeds 80 characters; contains 81 characters
35 | WARNING | Line exceeds 80 characters; contains 96 characters
--------------------------------------------------------------------------------

FILE: .../smart_trim-3351447/src/Plugin/Field/FieldFormatter/SmartTrimFormatter.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
--------------------------------------------------------------------------------
301 | WARNING | Only string literals should be passed to t() where possible
311 | WARNING | Only string literals should be passed to t() where possible
313 | WARNING | \Drupal calls should be avoided in classes, use dependency
| | injection instead
--------------------------------------------------------------------------------

FILE: ...ution/drupal8/web/modules/smart_trim-3351447/src/Truncate/TruncateHTML.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
36 | WARNING | The class short comment should describe what the class does and
| | not simply repeat the class name
--------------------------------------------------------------------------------

Time: 777ms; Memory: 10MB

Issue fork smart_trim-3351447

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

urvashi_vora created an issue. See original summary.

urvashi_vora’s picture

I am working on this, will provide a patch/MR shortly

urvashi_vora’s picture

Status: Needs work » Needs review
ultimike’s picture

Status: Needs review » Needs work

@urvashi_vora - thanks so much for doing this - just one tiny nit-pick above in comment 4...

-mike

avpaderno’s picture

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.

avpaderno’s picture

Priority: Normal » Minor
urvashi_vora’s picture

Assigned: urvashi_vora » Unassigned
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update
urvashi_vora’s picture

Hi @ultimike,

I have made the required changes, please verify.

Thanks

urvashi_vora’s picture

Hi @apaderno,

I have updated the issue summary, Please review.

Thanks

avpaderno’s picture

Status: Needs review » Needs work
+Issue #2782689 by yvesvanlaer, heddn, markie: How to 
+translate a custom read more link?
+Issue #2733381 by Stefdewa, Shreya Shetty: UTF-8 encoding 
+needed to show all characters correctly

It is quite hard to read the change log in that way. It would be better if the part of the line split by the previous line would be indented. I still think it would be better to leave those lines as they are, though, or change how they are formatted.

 /**
- * Class TruncateHTML.
+ * Implements Class TruncateHTML.
  */

That short description does not say what the class purpose is. (No, a class does not implement itself.)
Class is not spelled correctly, since it is not at the beginning of a sentence, it is not a proper noun nor an acronym.

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

elber’s picture

Status: Needs work » Needs review

Hi I just fixed the remaining errors, please revise.

avpaderno’s picture

Status: Needs review » Needs work
-Issue #2829817 by ultimike, markie, ruloweb: Apply dependency injection (DI) to TruncateHTML class
+Issue #2829817 by ultimike, markie, ruloweb: Apply dependency
+injection (DI) to TruncateHTML class

See my previous comment. What I reported there is still valid.

elber’s picture

Status: Needs work » Needs review

HI I did a rebase and changed the changelog file according with Apaderno's comment. Please revise.

markie’s picture

Status: Needs review » Needs work

the repository needs local rebase again to fix conflicts.

elber’s picture

Assigned: Unassigned » elber

I will do that

avpaderno’s picture

-Issue #2829817 by ultimike, markie, ruloweb: Apply dependency injection (DI) to TruncateHTML class
+Issue #2829817 by ultimike, markie, ruloweb: Apply dependency
+injection (DI) to TruncateHTML class

The splitted lines are still not indented, which means two spaces should be added to the second line.

elber’s picture

Assigned: elber » Unassigned
Status: Needs work » Needs review

Hi I just fixed the characters limit in the contributor.md. Please revise

markie’s picture

Whoops.. I approved but then found this:

☁  smart_trim [smart_trim-3351447-3351447-fix-the-issues] phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,js,info,txt,md,yml,twig .

FILE: /Users/mark/Sites/Development/drupal10/web/modules/development/smart_trim/contributor.md
----------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------
 170 | WARNING | Line exceeds 80 characters; contains 81 characters
----------------------------------------------------------------------------------------------

Time: 658ms; Memory: 12MB

  • markie committed 391a9ea9 on 2.0.x authored by urvashi_vora
    Issue #3351447 by elber, urvashi_vora, markie, ultimike, apaderno: Fix...
markie’s picture

Status: Needs review » Fixed

Updated the line that was at issue and merged. Thanks for your help!

avpaderno’s picture

+Issue #3131842 by markie, Kristen Pol, PapaGrande:
+Read more still not translated

Indenting means adding two spaces at the beginning of the line.
The second line is not indented at all, and that makes reading the list of changes done difficult to read.

Status: Fixed » Closed (fixed)

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