Problem/Motivation

FILE: ...ontrib/image_field_360/src/Plugin/Field/FieldFormatter/Image_field_360.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AND 2 WARNINGS AFFECTING 2 LINES
--------------------------------------------------------------------------------
23 | ERROR | Class name must use UpperCamel naming without underscores
295 | WARNING | t() calls should be avoided in classes, use
| | \Drupal\Core\StringTranslation\StringTranslationTrait and
| | $this->t() instead
295 | WARNING | Only string literals should be passed to t() where possible
--------------------------------------------------------------------------------

FILE: ...ution/drupal10/web/modules/contrib/image_field_360/image_field_360.install
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
24 | ERROR | [x] Expected newline after closing brace
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

Time: 453ms; Memory: 10MB

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

Proposed resolution

Fix all the issues and warnings for Drupal and DrupalPractice coding standards.

Remaining tasks

Patch review.

CommentFileSizeAuthor
#9 Screenshot from 2023-05-25 18-02-27.png99.25 KBraveen_thakur51
#8 interdiff-6_8.txt521 bytesurvashi_vora
#8 coding-standard-fixes-8.patch1.73 KBurvashi_vora
#6 interdiff-4_6.txt712 bytesurvashi_vora
#6 coding-standard-fixes-6.patch1.68 KBurvashi_vora
#4 interdiff-0_4.txt12.7 KBurvashi_vora
#4 coding-standard-fixes-4.patch1.9 KBurvashi_vora
coding_standard_fixes.patch12.19 KBurvashi_vora

Comments

urvashi_vora created an issue. See original summary.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Priority: Normal » Minor
Status: Needs review » Needs work

The src/Plugin/Field/FieldFormatter/Image_field_360.php file has been deleted, but no other file has been added. The error shown in the report does not say that file must be removed; it says the class name must be changed.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
urvashi_vora’s picture

urvashi_vora commented
Status: Needs work » Needs review
StatusFileSize
new coding-standard-fixes-4.patch1.9 KB
new interdiff-0_4.txt12.7 KB

Hi @apaderno,

For the feedback:

The src/Plugin/Field/FieldFormatter/Image_field_360.php file has been deleted, but no other file has been added. The error shown in the report does not say that file must be removed; it says the class name must be changed.

I appreciate your efforts in addressing the error regarding the class name and file naming convention. As per the error message, it was necessary to change the class name and subsequently rename the file to ensure that they match. I understand that during this process, the renamed file ended up being untracked in Git, while the deleted file remained tracked. Consequently, when checking the Git diff, only the deleted file was captured, excluding the changes made to the renamed file.

I have now updated the patch to include both changes. I kindly request you to verify the updated patch to ensure that both modifications are properly reflected. Thank you for your attention to detail and your cooperation in resolving this issue.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work 
- *   id = "Image_field_360",
+ *   id = "ImageField360",

It is not necessary to change the plugin ID. It is not required to match the class name; furthermore, plugin IDs use snail case. See AggregatorTitleFormatter.php, for example.

/**
 * Plugin implementation of the 'aggregator_title' formatter.
 *
 * @FieldFormatter(
 *   id = "aggregator_title",
 *   label = @Translation("Aggregator title"),
 *   description = @Translation("Formats an aggregator item or feed title with an optional link."),
 *   field_types = {
 *     "string"
 *   }
 * )
 */
class AggregatorTitleFormatter extends FormatterBase {
  // …
}

   /**
-   * Construct a Image_field_360 object.
+   * Construct a ImageField360 object.

Since that documentation comment is changed, also the description must be changed. The description for a constructor must start with Constructs a new followed by the class name (including its namespace), and end with object.

urvashi_vora’s picture

urvashi_vora commented
Status: Needs work » Needs review
StatusFileSize
new coding-standard-fixes-6.patch1.68 KB
new interdiff-4_6.txt712 bytes

Reverted plugin ID and added the word "new" in constructor comment.

Please review.

Thanks

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work

The class namespace is still missing, which is what the previous review reported as change too.

The description for a constructor must start with Constructs a new followed by the class name (including its namespace), and end with object.

urvashi_vora’s picture

urvashi_vora commented
Status: Needs work » Needs review
StatusFileSize
new coding-standard-fixes-8.patch1.73 KB
new interdiff-6_8.txt521 bytes

Please review.

raveen_thakur51’s picture

raveen_thakur51 commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Screenshot from 2023-05-25 18-02-27.png99.25 KB

@urvashi, after applying the #patch8, all the phpcs issues seem to be resolved.
As you can see in my attached screenshot.
Please & Thank You.

m.abdulqader’s picture

m.abdulqader commented

m.abdulqader’s picture

m.abdulqader commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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