Fix the issues reported by phpcs

Charchil Khandelwal → made their first commit to this issue’s fork.

I will review this.

Thanks @samit.310@gmail.com, patch #2 applied cleanly, all errors and warnings are fixed.
And i also created MR for same.
Please review.

In the StorageTypeForm class, the dependency injection of the string_translation service is missing.

Hi @mxh,

Thanks for the review, string_translation service has been added.

Please review.

-  /**
-   * {@inheritdoc}
-   */
-  public function postSave(EntityStorageInterface $storage, $update = TRUE) {
-    parent::postSave($storage, $update);
-
-    // if ($update && $this->getOriginalId() != $this->id()) {
-    //   $update_count = node_type_update_nodes($this->getOriginalId(), $this->id());
-    //   if ($update_count) {
-    //     \Drupal::messenger()->addStatus(\Drupal::translation()->formatPlural(
-    //       $update_count,
-    //       'Changed the storage type of 1 post from %old-type to %type.',
-    //       'Changed the storage type of @count posts from %old-type to %type.',
-    //       [
-    //         '%old-type' => $this->getOriginalId(),
-    //         '%type' => $this->id(),
-    //       ]
-    //     ));
-    //   }
-    // }
-    // if ($update) {
-    //   // Clear the cached field definitions as some settings affect the field
-    //   // definitions.
-    //   \Drupal::service('entity_field.manager')->clearCachedFieldDefinitions();
-    // }
-  }

I am not sure it is correct to remove that method. The commented out code could be code maintainers temporary commented out to then change it.

 /**
- * Class StorageSettingsForm.
+ * The StorageSettingsForm class.

That description is still repeating the class name without describing what the class purpose is.

-        // Automatically join to the storage table (or actually, storage_field_data).
+        // Automatically join to the storage table
+        // (or actually, storage_field_data).
         // Use a Views table alias to allow other modules to use this table too,
         // if they use the search index.

Avoiding that lines exceed 80 characters does not making shortening them to 50 characters. Also, when a comment line is shortened, all comment must be reformatted.

Please review patch.

Patch #10 applies cleanly and resolves all errors. I have attached the relevant screenshots.

Moving to RTBC.

+    $this->logger('content')->notice('Storage: deleted %name revision %revision.', [
+      '%name' => $this->revision->label(),
+      '%revision' => $this->revision->getRevisionId(),
+    ]);
+    $this->messenger()->addMessage($this->t('Revision from %revision-date of Storage %name has been deleted.', [
+      '%revision-date' => \Drupal::service('date.formatter')->format($this->revision->getRevisionCreationTime()),
+      '%name' => $this->revision->label(),
+    ]));

The code is still using \Drupal to inject dependencies.

 /**
- * Class StorageSettingsForm.
+ * The StorageSettingsForm class.

Like the existing comment, the changed comment is still repeating the class name.

+/**
+ * @file
+ * File description.
+ */
+

That does not describe the file purpose.
I suggest to check the description used by Drupal core for those files.

Updated patch #10 by addressing #12, please review it.

Thanks & Regards,
Mrinalini

@Mrinalini I applied your patch it applied successfully but still able to see some warning which can be fix by phpcbf.

Fixes issue mentioned in #9 and #12.
We can disable phpcs for StorageType::postSave() to fix violation we still have, or we can completely remove this method. Maintainer can always get the code from git history. I'd prefer removing postSave().

Hi @Nikolay Shapovalov,

Applied patch #17 successfully, however it threw 6 errors and 3 warnings.

storage git:(1.1.x) curl https://www.drupal.org/files/issues/2023-11-17/3334370-17.patch | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 23366  100 23366    0     0  57595      0 --:--:-- --:--:-- --:--:-- 59759
patching file config/schema/storage.schema.yml
patching file src/Access/StorageRevisionAccessCheck.php
patching file src/Controller/StorageController.php
patching file src/Entity/Storage.php
patching file src/Entity/StorageInterface.php
patching file src/Form/StorageForm.php
patching file src/Form/StorageRevisionDeleteForm.php
patching file src/Form/StorageRevisionRevertForm.php
patching file src/Form/StorageSettingsForm.php
patching file src/Form/StorageTypeForm.php
patching file src/StorageAccessControlHandler.php
patching file src/StorageListBuilder.php
patching file src/StoragePermissions.php
patching file src/StorageStorage.php
patching file src/StorageStorageInterface.php
patching file src/StorageViewsData.php
patching file storage.post_update.php
patching file storage.routing.yml
patching file storage.tokens.inc
➜  storage git:(1.1.x) ✗ cd ..
➜  contrib git:(main) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig storage

FILE: /Users/PrometInterns/Demo-site/drupal-orgissue/web/modules/contrib/storage/src/Entity/StorageType.php
-----------------------------------------------------------------------------------------------------------
FOUND 6 ERRORS AND 3 WARNINGS AFFECTING 8 LINES
-----------------------------------------------------------------------------------------------------------
 166 | WARNING | [ ] Possible useless method overriding detected
 169 | ERROR   | [x] Inline comments must start with a capital letter
 170 | WARNING | [ ] Line exceeds 80 characters; contains 85 characters
 170 | ERROR   | [ ] Comment indentation error, expected only 1 spaces
 172 | ERROR   | [ ] Comment indentation error, expected only 3 spaces
 173 | ERROR   | [ ] Comment indentation error, expected only 5 spaces
 175 | WARNING | [ ] Line exceeds 80 characters; contains 81 characters
 177 | ERROR   | [ ] Comment indentation error, expected only 7 spaces
 184 | ERROR   | [ ] Comment indentation error, expected only 1 spaces
-----------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------

Time: 1.12 secs; Memory: 12MB

Kindly check.

Thanks,
Jake

atul_ghate → made their first commit to this issue’s fork.

Added MR to fix all phpcs issue please.

Hi @atul_ghate,

Applied latest MR!16 version successfully, confirmed all issues fixed.

storage git:(1.1.x) ✗ curl https://git.drupalcode.org/project/storage/-/merge_requests/16.diff | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 26758    0 26758    0     0  66117      0 --:--:-- --:--:-- --:--:-- 67570
patching file config/schema/storage.schema.yml
patching file src/Access/StorageRevisionAccessCheck.php
patching file src/Controller/StorageController.php
patching file src/Entity/Storage.php
patching file src/Entity/StorageInterface.php
patching file src/Entity/StorageType.php
patching file src/Form/StorageForm.php
Hunk #1 succeeded at 22 with fuzz 2.
Hunk #2 FAILED at 36.
Hunk #3 FAILED at 65.
2 out of 3 hunks FAILED -- saving rejects to file src/Form/StorageForm.php.rej
patching file src/Form/StorageRevisionDeleteForm.php
patching file src/Form/StorageRevisionRevertForm.php
patching file src/Form/StorageSettingsForm.php
patching file src/Form/StorageTypeForm.php
patching file src/StorageAccessControlHandler.php
patching file src/StorageListBuilder.php
patching file src/StoragePermissions.php
patching file src/StorageStorage.php
patching file src/StorageStorageInterface.php
patching file src/StorageViewsData.php
patching file storage.post_update.php
patching file storage.routing.yml
patching file storage.tokens.inc
➜  storage git:(1.1.x) ✗ cd ..
➜  contrib git:(main) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig storage
➜  contrib git:(main) ✗

Will move this to RTBC.

Thanks,
Jake

apaderno → changed the visibility of the branch 3334370-drupal-coding-standards to hidden.

apaderno → changed the visibility of the branch 1.1.x to hidden.

GitLab CI no longer reports PHP_CodeSniffer warnings/errors. The MR fixes all the warnings/errors.