Phpcs issues

I will work on it.

there are no Fixable error of phpcs

OOps it's my mistake wrongly check so move to Active

Added updated patch and remove all PHPCS issues

-    // Load imagcover from settings if this is set.\Drupal::config('gifplayer.settings')->get('cover_png')
+    // Load imagcover from settings if this is set.\Drupal::config
+    // ('gifplayer.settings')->get('cover_png')

That is the wrong way to split code.
There is a missing space after the period.
The code after the period does not make much sense.

     if ($cid !== 0 && $file = $file_storage->load($cid)) {
-      $img_cover = \Drupal::service('file_url_generator')->generateAbsoluteString($file->getFileUri());
+      $img_cover = $this->service('file_url_generator')->generateAbsoluteString($file->getFileUri());
     }

There is no service() method.

I am having this issue after I applied the patch and tried to add a new "Gif Player" field:

TypeError: Drupal\gifplayer\Plugin\Field\FieldFormatter\GifPlayerFormatter::__construct(): Argument #3 ($field_definition) must be of type Drupal\gifplayer\Plugin\Field\FieldFormatter\FieldDefinitionInterface, Drupal\field\Entity\FieldConfig given, called in /var/www/html/web/modules/contrib/gifplayer/src/Plugin/Field/FieldFormatter/GifPlayerFormatter.php on line 104 in Drupal\gifplayer\Plugin\Field\FieldFormatter\GifPlayerFormatter->__construct() (line 71 of /var/www/html/web/modules/contrib/gifplayer/src/Plugin/Field/FieldFormatter/GifPlayerFormatter.php)

   /**
+   * File url generator object.
+   *

+   * @param \Drupal\Core\File\FileUrlGenerator $fileUrlGenerator
+   *   File url generator object.

url in the description is misspelled. Since it is an acronym, it must be spelled in capital letters.
The definite article is missing at the beginning of the description.

+  /**
+   * The list of available modules.
+   *
+   * @var \Drupal\Core\Extension\ModuleExtensionList
+   */
+  protected $extensionListModule;

+   * @param \Drupal\Core\Extension\ModuleExtensionList $extension_list_module
+   *   The list of available modules.

The description Drupal core use for that property/parameter is The module extension list.

+  /**
+   * Configuration Factory.
+   *
+   * @var \Drupal\Core\Config\ConfigFactory

+   * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
+   *   Config Factory Interface.

A definite article is missing from the descriptions.
Only the first word must be capitalized.

+  /**
+   * Construct a MyFormatter object.
+   *

The description for a constructor must start with Constructs a new followed by the class name (including its namespace), and end with object.
The class name is not correct, since that comment is for the GifPlayerFormatter class.

+   * @param string $plugin_id
+   *   The plugin_id for the plugin instance.

plugin_id must be plugin ID.

+   * @param \Drupal\Core\Field\FieldDefinitionInterface $field_definition
+   *   Defines an interface for entity field definitions.

That description is not correct: An object does not define an interface, but its class implements an interface.
The correct description is The definition of the field to which the formatter is associated.

+  public function __construct(
+    $plugin_id,
+    $plugin_definition,
+    FieldDefinitionInterface $field_definition,
+    array $settings,
+    $label,
+    $view_mode,
+    array $third_party_settings,
+    FileUrlGenerator $fileUrlGenerator,
+    ModuleExtensionList $extension_list_module,
+    ConfigFactoryInterface $configFactory
+    ) {

As per Drupal coding standards, a function/method declaration must be written on a single line.

-    // Load imagcover from settings if this is set.\Drupal::config('gifplayer.settings')->get('cover_png')
+    // Load imagcover from settings if this is set.
+    // \Drupal::config('gifplayer.settings')->get('cover_png').
     // @codingStandardsIgnoreStart

That comment is not necessary, as it describe what is already clear from the code. Instead of being edited, it should be removed.

+    $cid = ($this->configFactory->get('gifplayer.settings')->get('cover_png') !== NULL)
+      ? $this->configFactory->get('gifplayer.settings')->get('cover_png')
+      : 0;

Code lines are allowed to exceed 80 characters, if they are easier to understand. Writing that code in a single line makes it easier to understand.

Addressed #15 and raised a MR. Please review.

Hi @bharath-kondeti,

I applied the latest MR !2 to the Gif Player Field module against 8.x-1.x-dev on Drupal 9.5.10 and I'm still seeing 1 warning from GifPlayerFormatter.php file, please see below:

I ran this command on the module:
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig gifplayer/

Please see the attached files for reference.

I'm moving this to Needs work for now.

Thank you!

We can ignore that warning. @roberttabigue. Also, I've fixed the class name in the constructor description which was incorrect.

Hi @nitin_lama @apaderno thanks for the work on this, could you provide a patch file instead, if possible? I am having an issue with the merge pipeline.

Addressing comment #27. Providing updated patch.

Thanks @nitin_lama , I applied the patch with no issues, committed to the branch 8.x-1.x: https://www.drupal.org/project/gifplayer/releases/8.x-1.x-dev →

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

danrod → closed merge request !2