Problem/Motivation

When there are failing tests maintainership is slightly more difficult because you can't depend on the tests to catch new issues.

Steps to reproduce

Run PHPCS on the repo. See https://addons.ddev.com/addons/ddev/ddev-drupal-contrib for help.

Proposed resolution

Address everything mentioned in this report.


FILE: /var/www/html/web/modules/custom/easy_breadcrumb/easy_breadcrumb.install
------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------
 153 | ERROR | [x] Expected 1 newline at end of file; 0 found (Drupal.Files.EndFileNewline.NoneFound)
------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/custom/easy_breadcrumb/src/TitleResolver.php
---------------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AND 1 WARNING AFFECTING 3 LINES
---------------------------------------------------------------------------------------------------------------------------------------------
  74 | WARNING | [ ] Line exceeds 80 characters; contains 91 characters (Drupal.Files.LineLength.TooLong)
 103 | ERROR   | [x] Expected 1 blank line after function; 0 found (Squiz.WhiteSpace.FunctionSpacing.AfterLast)
 104 | ERROR   | [x] The closing brace for the class must have an empty line before it (Drupal.Classes.ClassDeclaration.CloseBraceAfterBody)
---------------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/custom/easy_breadcrumb/tests/src/Kernel/EasyBreadcrumbBuilderTest.php
---------------------------------------------------------------------------------------------------------------------------------------------
FOUND 7 ERRORS AND 2 WARNINGS AFFECTING 8 LINES
---------------------------------------------------------------------------------------------------------------------------------------------
 231 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4 (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
 232 | ERROR   | [x] Expected 5 space(s) before asterisk; 3 found (Drupal.Commenting.DocCommentAlignment.SpaceBeforeStar)
 232 | WARNING | [ ] Line exceeds 80 characters; contains 94 characters (Drupal.Files.LineLength.TooLong)
 233 | ERROR   | [x] Expected 5 space(s) before asterisk; 3 found (Drupal.Commenting.DocCommentAlignment.SpaceBeforeStar)
 237 | ERROR   | [x] TRUE, FALSE and NULL must be uppercase; expected "TRUE" but found "true" (Generic.PHP.UpperCaseConstant.Found)
 272 | WARNING | [ ] Line exceeds 80 characters; contains 92 characters (Drupal.Files.LineLength.TooLong)
 277 | ERROR   | [x] TRUE, FALSE and NULL must be uppercase; expected "TRUE" but found "true" (Generic.PHP.UpperCaseConstant.Found)
 312 | ERROR   | [x] Expected 1 blank line after function; 0 found (Squiz.WhiteSpace.FunctionSpacing.AfterLast)
 313 | ERROR   | [x] The closing brace for the class must have an empty line before it (Drupal.Classes.ClassDeclaration.CloseBraceAfterBody)
---------------------------------------------------------------------------------------------------------------------------------------------

Remaining tasks

Add a merge request.
Update gitlab-ci.yml to disallow merging if tests are failing.

User interface changes

N/A

Introduced terminology

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

Issue fork easy_breadcrumb-3501555

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

trackleft2 created an issue. See original summary.

trackleft2 opened merge request !143

trackleft2’s picture

trackleft2
Primary language English
Location Tucson, AZ 🇺🇸
commented
Issue summary: View changes
trackleft2’s picture

trackleft2
Primary language English
Location Tucson, AZ 🇺🇸
commented

This merge request is blocked by https://www.drupal.org/project/easy_breadcrumb/issues/3501451 because the merge request there deletes a file that this MR relies on being gone.

Once that merge request is merged, this fork's codebase should be updated. that will make all of the tests pass if AFAICT

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

svendecabooter’s picture

svendecabooter
he/him
Primary language Dutch
Location Gent
commented
Status: Active » Needs review

Merge request has been updated, since the blocking issue is now resolved.
PHPCS pipeline is passing now.

loopduplicate’s picture

loopduplicate commented
Assigned: Unassigned » loopduplicate
Status: Needs review » Needs work

Thanks all for the help on this. The MR was a little outdated. I'll be creating a new MR for this. It's simpler than MR143. I also found particular things potentially problematic with MR143, like the use of // phpcs:disable and the shortening of commments like "load the entity if present and use that field" to "use it", instead of just wrapping the comment.

loopduplicate opened merge request !157

loopduplicate’s picture

loopduplicate commented
Issue summary: View changes
loopduplicate’s picture

loopduplicate commented
Assigned: loopduplicate » Unassigned
Status: Needs work » Needs review

OK, all set. Back to Needs review.

trackleft2’s picture

trackleft2
Primary language English
Location Tucson, AZ 🇺🇸
commented
Status: Needs review » Reviewed & tested by the community

LGTM Thanks!