Problem/Motivation

Coding standards were addressed in #3339626: Fix the issues reported by phpcs. However, a later commit introduced an issue:

FILE: ...pi_resources/src/Unstable/Controller/ArgumentResolver/DocumentResolver.php
--------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------
 37 | ERROR | [x] Data types in @param tags need to be fully namespaced
    |       |     (Drupal.Commenting.DataTypeNamespace.DataTypeNamespace)
 39 | ERROR | [x] Data types in @param tags need to be fully namespaced
    |       |     (Drupal.Commenting.DataTypeNamespace.DataTypeNamespace)
--------------------------------------------------------------------------------

Let's fix this and require phpcs to pass so that future MRs don't deviate from the coding standards.

CommentFileSizeAuthor
#5 jsonapi_resources-3461125-result_of_phpcs.png21.24 KBnilesh.addweb
#5 jsonapi_resources-3461125-applied-patch.png52.78 KBnilesh.addweb

Issue fork jsonapi_resources-3461125

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

ptmkenny created an issue. See original summary.

ptmkenny opened merge request !18

ptmkenny’s picture

ptmkenny commented
Status: Active » Needs review

This MR additionally updates the GitLab CI template to the latest version (removes the giant Drupal logo, etc.; only cosmetic changes).

ptmkenny’s picture

ptmkenny commented
Component: Documentation » Code
nilesh.addweb’s picture

nilesh.addweb commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new jsonapi_resources-3461125-applied-patch.png52.78 KB
new jsonapi_resources-3461125-result_of_phpcs.png21.24 KB

I have applied the patch cleanly, and the above errors are resolved.
Please check the Screenshots for the same.

ptmkenny’s picture

ptmkenny commented

@nilesh.addweb Thanks for testing.

In the future, for GitLab CI issues, you can see the results of applying the MR by clicking the checkmark icon next to the MR, which will show you the pipelines, including the phpcs test.

ptmkenny’s picture

ptmkenny commented

Coding standards were updated today, so I updated the MR to incorporate those changes.

ptmkenny’s picture

ptmkenny commented
Title: Fix phpcs test again and require it to pass so that it doesn't break again » Fix phpcs test (address PHP 8.4 deprecations) and require it to pass

This module has some PHP 8.4 deprecations, which are already addressed in the MR, so I'm changing the title because PHP 8.4 has now been released.

ptmkenny’s picture

ptmkenny commented
Priority: Minor » Major

Bumping to major because this can block sites on PHP 8.4.

acbramley’s picture

acbramley commented

Not a fan of conflating deprecation fixes for PHP 8.4 with other coding standards cruft, but this would be good to get in.

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

  • mglaman committed 5ea3b69c on 8.x-1.x authored by ptmkenny 
    Issue #3461125: Fix phpcs test (address PHP 8.4 deprecations) and...
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Not a fan of conflating deprecation fixes for PHP 8.4 with other coding standards cruft, but this would be good to get in

Normally I agree, but given it's just adding ? for nullables, I think it is okay.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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