Fix the issues reported by phpcs

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

I will review patch.

Thanks @samit.310@gmail.com, patch #2 applied cleanly, all errors and warnings are fixed.

Created MR for this.
Please review.

   return [
     2 => new DeliveryCandidate(['subscribe_node'], ['sms'], 2),
-    7 => new DeliveryCandidate(['subscribe_og', 'subscribe_user'], ['sms', 'email'], 7),
+    7 => new DeliveryCandidate([
+      'subscribe_og',
+      'subscribe_user',
+    ], [
+      'sms',
+      'email',
+    ], 7),
   ];

Code lines are allowed to exceed 80 characters, if they are more readable. The existing code is more readable.

+  /**
+   * {@inheritdoc}
+   */
+  public function __construct(ConfigFactoryInterface $config_factory, Manager $notifier_manager) {

{@inheritdoc} is not used for constructors.

Updated patch with recommendation from #8

Please review the patch #10

Thanks!

+  /**
+   * Constructs a new MessageSubscribeAdminSettings object.
+   */
+  public function __construct(ConfigFactoryInterface $config_factory, Manager $notifier_manager) {

The documentation comment for a constructor must document its parameters.

 dependencies:
-  - message_subscribe
+  - drupal:message_subscribe

The Message Subscribe → module (this very module) is not a Drupal core module.

Updated patch based on #14. Please review.

+  /**
+   * Constructs a new MessageSubscribeAdminSettings object.
+   *

The class namespace is missing.

+   * @param Drupal\Core\Config\ConfigFactoryInterface $config_factory
+   *   The Config Factory contains all the configs.

The config factory. is sufficient.

+   * @param Drupal\message_notify\Plugin\Notifier\Manager $notifier_manager
+   *   Notification Manager of message_notify.

That description is missing a definite article.
Notification and Manager are misspelled, since they need to be spelled with lowercase letters.
There is no need to say which module implements that class, nor say its machine name.

Hi,

Confirmed no PHPCS errors after applying Patch #17 and the latest MR to the Message Subscribe module with version 8.x-1.x-dev and with Drupal core version 9.5.8.

Moving this now to RTBC.
Kindly refer to the attached screenshots, please.

  * Implements hook_message_subscribe_get_subscribers_alter().
  *
  * Alter the subscribers list.

The last line is not necessary too.

Patch updated.
Please review.

+      /*
+       * Can not remove $j, as we have to use it with foreach.
+       * // @codingStandardsIgnoreStart
+       */
       foreach (range(1, 5) as $j) {
         $this->terms[] = $this->createTerm($vocab);
       }
+      // @codingStandardsIgnoreEnd

There is no need to use @codingStandardsIgnoreStart. The following code does not cause any warning/error.

       for ($j = 1; $j <= 5; $j++) {
         $this->terms[] = $this->createTerm($vocab);
       }

Patch updated.
Please review.

Cannot apply patch Fix the issues reported by phpcs ( https://www.drupal.org/files/issues/2023-05-22/3331903-27.patch → )!

Checking patch message_subscribe.api.php...
Checking patch message_subscribe_email/message_subscribe_email.info.yml...
Checking patch message_subscribe_email/message_subscribe_email.module...
Checking patch message_subscribe_email/tests/src/Functional/ViewsTest.php...
Checking patch message_subscribe_email/tests/src/FunctionalJavascript/MessageSubscribeEmailTest.php...
Checking patch message_subscribe_example/README.md...
Checking patch message_subscribe_example/message_subscribe_example.module...
error: while searching for:
  // a custom list (see hook_user_insert() below). This is just an
  // illustration of another solution.
  $query = \Drupal::entityQuery('user')
  ->condition('status', 1);
  $query->condition('roles', 'administrators');
  $admin_uids = $query->execute();
  foreach ($admin_uids as $uid) {

error: patch failed: message_subscribe_example/message_subscribe_example.module:52
error: message_subscribe_example/message_subscribe_example.module: patch does not apply
Checking patch message_subscribe_ui/src/Controller/SubscriptionController.php...
Checking patch message_subscribe_ui/src/Plugin/Block/Subscriptions.php...
Checking patch message_subscribe_ui/tests/src/Functional/SubscriptionsBlockTest.php...
Checking patch src/Form/MessageSubscribeAdminSettings.php...
Hunk #2 succeeded at 83 (offset 6 lines).
Checking patch src/Subscribers.php...
Hunk #2 succeeded at 243 (offset 4 lines).
Hunk #3 succeeded at 286 (offset 12 lines).
Checking patch src/SubscribersInterface.php...
Checking patch tests/modules/message_subscribe_test/message_subscribe_test.info.yml...
Checking patch tests/src/Functional/MenuTest.php...
Checking patch tests/src/Functional/UninstallTest.php...
Checking patch tests/src/Kernel/SubscribersTest.php...
Checking patch tests/src/Unit/Subscribers/DeliveryCandidateTest.php...
Checking patch tests/src/Unit/SubscribersTest.php...

-    7 => new DeliveryCandidate(['subscribe_og', 'subscribe_user'], ['sms', 'email'], 7),
+    7 => new DeliveryCandidate(
+      ['subscribe_og', 'subscribe_user'],
+      ['sms', 'email'],
+      7),
   ];

-    ['@flags' => implode(', ', $flag_ids), '@uids' => implode(', ', array_keys($uids))]
+    [
+      '@flags' => implode(', ', $flag_ids),
+      '@uids' => implode(', ', array_keys($uids)),
+    ]

Code lines are allowed to exceed 80 characters, if they are easier to understand. The existing code is easier to understand.

-      $this->assertSession()->pageTextContains(t('You are not subscribed to any items.'));
+      $this->assertSession()->pageTextContains($this->t('You are not subscribed to any items.'));
     }

Tests should not use translatable strings simply for the fact that they are not translated, when tests run.

This means that the following change must be reverted.

   use ContentTypeCreationTrait;
   use NodeCreationTrait;
+  use StringTranslationTrait;

bharath-kondeti → made their first commit to this issue’s fork.

Raised a new MR https://git.drupalcode.org/project/message_subscribe/-/merge_requests/7 with latest patch and addressed #29

I updated the description with the errors reported from the Gitlab CI pipeline, 📌 GitLab CI Needs review

I am working i branch 3331903-phpcs-eslint

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