Contrib.social

PHPCS errors found after creating sub-theme

Open on Drupal.org →
Created on 14 November 2023, about 1 year ago
Updated 28 February 2024, 11 months ago

Problem/Motivation

When running PHPCS in our builds for the Governor, we ran into one mild issue with some doc commenting.

Steps to reproduce

Run PHPCS in your build, find the following:

+ vendor/bin/phpcs
.............................E.............................. 60 / 62 (97%)
..                                                           62 / 62 (100%)
FILE: ...tlassian/pipelines/agent/build/web/themes/custom/mytheme/mytheme.theme
----------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 43 | ERROR | [x] There must be exactly one blank line before the
    |       |     tags in a doc comment
    |       |     (Drupal.Commenting.DocComment.SpacingBeforeTags)
 43 | ERROR | [x] Trailing punctuation for @see references is not
    |       |     allowed.
    |       |     (Drupal.Commenting.FunctionComment.SeePunctuation)
 45 | ERROR | [ ] Parameter tags must be defined first in a doc
    |       |     comment
    |       |     (Drupal.Commenting.DocComment.ParamNotFirst)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
Time: 366ms; Memory: 14MB

Proposed resolution

Create and test patch to change the comments to avoid php code sniffer errors.

💬 Support request
Status

RTBC

Version

1.0

Component

Code

Created by

🇺🇸United States saltednut Chicago

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @saltednut
  • Comment about 1 year ago →
    🇺🇸United States saltednut Chicago
  • Comment about 1 year ago →
    🇺🇸United States saltednut Chicago
  • @saltednut opened merge request.
  • Comment about 1 year ago →
    🇺🇸United States saltednut Chicago

    As a followup, there is also PHPStan issue that we found but it'll require either rewriting a function or just renaming it in the sub-theme when one is generated.

    PHP Fatal error: Cannot redeclare _is_page_link_visible() (previously declared in /Users/myuser/Work/client/web/themes/contrib/governor/governor.theme:55) in /Users/myuser/Work/client/web/themes/custom/client/client.theme on line 70

    The workaround, for now, is to rename the global function inside client.theme - but this could be fixed permanently perhaps if a service was provided by the base theme. If needed, I can file another issue for that.

  • Comment about 1 year ago →
    🇮🇳India Indranil Roy kolkata

    Hey @saltednut, I have added the patch please verify it.

  • Status changed to Needs review about 1 year ago
  • Comment about 1 year ago →
    🇮🇳India Indranil Roy kolkata
  • Comment about 1 year ago →
    🇺🇸United States saltednut Chicago

    Thanks @Indranil Roy, there is a merge request pending so we may not need a patch?

  • Status changed to RTBC 11 months ago
  • Comment 11 months ago →
    🇺🇸United States kurttrowbridge

    Hello! I tested this, first confirming that I saw the PHPCS errors mentioned when the patch was not yet applied. After applying the MR as a patch, the PHPCS errors are no longer present. Marking RTBC as a result.

    (That said, I should note that line 60 in the .theme file—the comment starting with "Calculate the minimum and maximum"—warns me, both before and after, that it's lengthier than the 80-character limit in Drupal's standards. Not sure why it doesn't appear in your testing, so I didn't flip back to Needs Work in case it's just something I'm doing differently on my end.)

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024