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!

loopduplicate’s picture

loopduplicate commented

Just bumping this. It would be nice to get this in. For example, the automated Drupal 12 issue keeps flagging some things that are fixed in this MR. Once this gets in, all tests will pass. Wouldn't that be nice? :3

loopduplicate opened merge request !172

loopduplicate changed the visibility of the branch 3501555-8 to hidden.

loopduplicate changed the visibility of the branch 3501555-address-phpcs-errors to hidden.

loopduplicate changed the visibility of the branch 2.x to hidden.

loopduplicate’s picture

loopduplicate commented

OK, new MR in #13. It has the same code from the original request, but it's up to date with 2.x-dev and it has one more fix that was introduced with the addition of tugboat, which was just a missing new line at the end of the file.

loopduplicate’s picture

loopduplicate commented
Title: Address PHPCS errors in gitlab-ci jobs » Address PHPCS errors

loopduplicate opened merge request !183

  • loopduplicate committed 5b21e35c on 2.x 
    Issue: #3501555 Address PHPCS errors

By: trackleft2
By: svendecabooter...
loopduplicate’s picture

loopduplicate commented
Status: Reviewed & tested by the community » Fixed

Thanks @trackleft2 and @svendecabooter

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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