- Issue created by @indrapatil
- Assigned to indrapatil
- Status changed to Needs work
almost 2 years ago 5:09pm 19 February 2023 - Issue was unassigned.
- Status changed to Active
almost 2 years ago 9:26pm 20 February 2023 - Status changed to Needs review
almost 2 years ago 5:27am 21 February 2023 - 🇮🇳India Akram Khan Cuttack, Odisha
OOps it's my mistake wrongly check so move to Active
- Status changed to Active
almost 2 years ago 5:41am 21 February 2023 - Status changed to Needs review
almost 2 years ago 5:42am 21 February 2023 - 🇮🇳India Akram Khan Cuttack, Odisha
Added updated patch and remove all PHPCS issues
- Status changed to Needs work
almost 2 years ago 8:33am 21 February 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
- // 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. - Status changed to Needs review
over 1 year ago 7:55am 22 February 2023 - Assigned to danrod
- 🇨🇦Canada danrod Ottawa
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)
- Issue was unassigned.
- Status changed to Needs work
over 1 year ago 10:19am 30 July 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
/** + * 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 withobject.
The class name is not correct, since that comment is for theGifPlayerFormatter
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.
- First commit to issue fork.
- @bharath-kondeti opened merge request.
- Status changed to Needs review
over 1 year ago 1:51pm 30 July 2023 - 🇮🇳India bharath-kondeti Hyderabad
Addressed #15 and raised a MR. Please review.
- Status changed to Needs work
over 1 year ago 4:52pm 31 July 2023 - 🇵🇭Philippines roberttabigue
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!
- Assigned to nitin_lama
- Status changed to Needs review
over 1 year ago 8:52am 4 August 2023 - 🇮🇳India nitin_lama India
We can ignore that warning. @roberttabigue. Also, I've fixed the class name in the constructor description which was incorrect.
- Issue was unassigned.
- Status changed to Needs work
over 1 year ago 8:36am 6 August 2023 - Assigned to nitin_lama
- Issue was unassigned.
- Status changed to Needs review
over 1 year ago 11:05am 7 August 2023 - Assigned to danrod
- 🇨🇦Canada danrod Ottawa
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.
- Status changed to RTBC
about 1 year ago 11:58pm 28 August 2023 - Assigned to nitin_lama
- Issue was unassigned.
- 🇨🇦Canada danrod Ottawa
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 →
- Status changed to Fixed
about 1 year ago 4:57pm 29 August 2023 Automatically closed - issue fixed for 2 weeks with no activity.