Problem/Motivation

FILE: ...wdalmnb/src/Plugin/Field/FieldFormatter/CloudflareVideoFormatter.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
--------------------------------------------------------------------------
140 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
141 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
142 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
143 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
--------------------------------------------------------------------------
CommentFileSizeAuthor
#13 interdiff_9-13.txt4.54 KBtim-diels
#13 3236359-remove_drupal_calls-13.patch5.32 KBtim-diels
#9 3236359-9.patch4.18 KBmohit.bansal623

Issue fork cloudflare_stream-3236359

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

pjbaert created an issue. See original summary.

pjbaert’s picture

pjbaert
Primary language Dutch
commented
Parent issue: » #3236350: Fix Codesniffer issues

mohit.bansal623 made their first commit to this issue’s fork.

mohit.bansal623 opened merge request !11

mohit.bansal623’s picture

mohit.bansal623 commented
Status: Active » Needs review

Please review the merge request.

tim-diels’s picture

tim-diels
He/Him
Primary language Dutch
Location Belgium 🇧🇪
commented
Status: Needs review » Needs work

Please only do the needed work in this issue and not copy over your whole changes from other issues. We're not reviewing this untill this is clearly only addressing the needed work.

mohit.bansal623’s picture

mohit.bansal623 commented
Status: Needs work » Needs review

This file only contains changes related to dependency injection calls as per the requirement.

tim-diels’s picture

tim-diels
He/Him
Primary language Dutch
Location Belgium 🇧🇪
commented
Status: Needs review » Needs work

Please see review, needs work.

mohit.bansal623’s picture

mohit.bansal623 commented
StatusFileSize
new 3236359-9.patch4.18 KB

Uploading patch, please review.

mohit.bansal623’s picture

mohit.bansal623 commented
Status: Needs work » Needs review
tim-diels’s picture

tim-diels
He/Him
Primary language Dutch
Location Belgium 🇧🇪
commented
Status: Needs review » Needs work 
  1. +++ b/src/Plugin/Field/FieldFormatter/CloudflareVideoFormatter.php
@@ -23,6 +28,62 @@ use GuzzleHttp\Exception\GuzzleException;
+  public function __construct(ConfigFactoryInterface $configFactory, MessengerInterface $messenger, ClientInterface $http_client, UuidInterface $uuid) {

    Missing parent constructor call 

  2. +++ b/src/Plugin/Field/FieldFormatter/CloudflareVideoFormatter.php
@@ -23,6 +28,62 @@ use GuzzleHttp\Exception\GuzzleException;
+  public static function create(ContainerInterface $container) {

    Declaration must be compatible with ContainerFactoryPluginInterface::create(container: \Symfony\Component\DependencyInjection\ContainerInterface, configuration: array, plugin_id: string, plugin_definition: mixed)

    Declaration must be compatible with FormatterBase::create(container: \Symfony\Component\DependencyInjection\ContainerInterface, configuration: array, plugin_id: string, plugin_definition: mixed) 

  3. +++ b/src/Plugin/Field/FieldFormatter/CloudflareVideoFormatter.php
@@ -23,6 +28,62 @@ use GuzzleHttp\Exception\GuzzleException;
+      $container->get('config.factory'), $container->get('messenger'), $container->get('http_client'), $container->get('uuid')

    Please use the correct formatting here

tim-diels’s picture

tim-diels
He/Him
Primary language Dutch
Location Belgium 🇧🇪
commented
Assigned: Unassigned » tim-diels
tim-diels’s picture

tim-diels
He/Him
Primary language Dutch
Location Belgium 🇧🇪
commented
Status: Needs work » Needs review
StatusFileSize
new 3236359-remove_drupal_calls-13.patch5.32 KB
new interdiff_9-13.txt4.54 KB

Rework done on previous patch.

tim-diels’s picture

tim-diels
He/Him
Primary language Dutch
Location Belgium 🇧🇪
commented
Assigned: tim-diels » Unassigned

  • pjbaert committed 56549e7 on 8.x-2.x 
    Issue #3236359 by mohit.bansal623, tim-diels, pjbaert: Remove \Drupal...
pjbaert’s picture

pjbaert
Primary language Dutch
commented
Status: Needs review » Fixed

Beautifully done. This was pushed to the 2.x branch
Thanks for all your work!

Status: Fixed » Closed (fixed)

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

pjbaert closed merge request !11