Address warnings from gitlab

Created on 5 April 2023, over 1 year ago
Updated 11 June 2024, 5 months ago

Fixed the issues reported by phpcs

📌 Task
Status

Needs work

Version

2.0

Component

Code

Created by

Live updates comments and jobs are added and updated live.
  • Coding standards

    It involves compliance with, or the content of coding standards. Requires broad community agreement.

  • Novice

    It would make a good project for someone who is new to the Drupal contribution process. It's preferred over Newbie.

Sign in to follow issues

Comments & Activities

  • Issue created by @mukesh88
  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • Status changed to Needs work over 1 year ago
  • 🇮🇳India TanujJain-TJ

    Tested and verified patch #2 on drupal:10.1.x, the patch applied successfully but still getting some PHPCS errors after running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml

    need to fix below mentioned errors, chaning it to needs work again.

    FILE: /entity_print/src/Asset/AssetCollector.php
    --------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
    --------------------------------------------------------------------------------
     51 | WARNING | \Drupal calls should be avoided in classes, use dependency
        |         | injection instead
     52 | WARNING | The removal-version 'entity_print:3.0.0' does not match the
        |         | lower-case machine-name standard: drupal:n.n.n or
        |         | project:n.x-n.n
    --------------------------------------------------------------------------------
    
    
    FILE: entity_print/src/Asset/AssetRenderer.php
    --------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    --------------------------------------------------------------------------
     71 | WARNING | Variable $build inside unset call is undefined.
    --------------------------------------------------------------------------
  • Assigned to sakthi_dev
  • Issue was unassigned.
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
    1. +++ b/src/PrintBuilder.php
      @@ -42,11 +49,14 @@ class PrintBuilder implements PrintBuilderInterface {
           $this->stringTranslation = $string_translation;
      +    $this->fileSystem = $file_system;
      

      We need a deprecation layer here, so default this to null, and trigger_error if the value is null, then fetch from the singleton

      See \Drupal\entity_print\Asset\AssetCollector::__construct for an example

      Will need a new change record to go with that, so you can use it in the message. The link to create a change record is at the top of this page

    2. +++ b/tests/modules/entity_print_test/src/Plugin/EntityPrint/PrintEngine/TestPrintEngine.php
      @@ -65,8 +65,18 @@ class TestPrintEngine extends PrintEngineBase {
      +    $this->configuration['test_engine_setting'] = $form_state->getValue(
      +      [
      +        'testprintengine',
      +        'test_engine_setting',
      +      ]
      +    );
      +    $this->configuration['test_engine_suffix'] = $form_state->getValue(
      +      [
      +        'testprintengine',
      +        'test_engine_suffix',
      +      ]
      +    );
      
      +++ b/tests/modules/entity_print_test/src/Plugin/EntityPrint/PrintEngine/TestWordPrintEngine.php
      @@ -74,7 +74,12 @@ class TestWordPrintEngine extends PrintEngineBase {
      -    $this->configuration['test_word_setting'] = $form_state->getValue(['test_word_print_engine', 'test_word_setting']);
      +    $this->configuration['test_word_setting'] = $form_state->getValue(
      +      [
      +        'test_word_print_engine',
      +        'test_word_setting',
      +      ]
      +    );
      
      +++ b/tests/src/FunctionalJavascript/EntityPrintAdminTest.php
      @@ -101,7 +101,13 @@ class EntityPrintAdminTest extends WebDriverTestBase {
      -    $this->assertEquals(['test_engine_setting' => 'testvalue', 'test_engine_suffix' => 'overridden'], $config_entity->getSettings());
      +    $this->assertEquals(
      +      [
      +        'test_engine_setting' => 'testvalue',
      +        'test_engine_suffix' => 'overridden',
      +      ],
      +      $config_entity->getSettings()
      +    );
      

      can we collapse these a bit, the [] can go on the same lines as the ()

    The items at #4 are false positives and need to be ignored by adding some ignore rules per https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#ignorin...

  • @larowlan #7 point 2 have done with added phpcs ignore and recreate patch. I checked for point 1 \Drupal\entity_print\Asset\AssetCollector::__construct

    object is InfoParserInterface then @trigger_error for deprecation so in PrintBuilder class which object will check null for @trigger_error.

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    the new object (file system)

  • @larowlan could you please check the below code for #7 point 1

    +   * @param \Drupal\Core\File\FileSystemInterface $file_system
    +   *   The file system service.
        */
    -  public function __construct(RendererFactoryInterface $renderer_factory, EventDispatcherInterface $event_dispatcher, TranslationInterface $string_translation) {
    +  public function __construct(RendererFactoryInterface $renderer_factory, EventDispatcherInterface $event_dispatcher, TranslationInterface $string_translation, FileSystemInterface $file_system = NULL) {
         $this->rendererFactory = $renderer_factory;
         $this->dispatcher = $event_dispatcher;
         $this->stringTranslation = $string_translation;
    +    if (!$file_system) {
    +      @trigger_error('Calling $file_system without argument the constructor of ' . __CLASS__ . ' is deprecated in entity_print:8.x-2.4 and is removed from entity_print:3.0.0. Pass an instance of FileSystemInterface instead. See https://www.drupal.org/node/3238430', E_USER_DEPRECATED);
    +      $this->fileSystem = \Drupal::service('file_system');
    +    }
    +    $this->fileSystem = $file_system;
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Ah sorry, that was a bad example, because it was dealing with a replaced paramater.

    Can you check \Drupal\Core\Ajax\AjaxResponseAttachmentsProcessor::__construct in core for a better example

  • @larowlan I checked file core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php it isn't good example for comment #11

     /**
       * Constructs an AjaxResponseAttachmentsProcessor object.
       *
       * @param \Drupal\Core\Asset\AssetResolverInterface $asset_resolver
       *   An asset resolver.
       * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
       *   A config factory for retrieving required config objects.
       * @param \Drupal\Core\Asset\AssetCollectionRendererInterface $css_collection_renderer
       *   The CSS asset collection renderer.
       * @param \Drupal\Core\Asset\AssetCollectionRendererInterface $js_collection_renderer
       *   The JS asset collection renderer.
       * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack
       *   The request stack.
       * @param \Drupal\Core\Render\RendererInterface $renderer
       *   The renderer.
       * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
       *   The module handler.
       */
      public function __construct(AssetResolverInterface $asset_resolver, ConfigFactoryInterface $config_factory, AssetCollectionRendererInterface $css_collection_renderer, AssetCollectionRendererInterface $js_collection_renderer, RequestStack $request_stack, RendererInterface $renderer, ModuleHandlerInterface $module_handler) {
        $this->assetResolver = $asset_resolver;
        $this->config = $config_factory->get('system.performance');
        $this->cssCollectionRenderer = $css_collection_renderer;
        $this->jsCollectionRenderer = $js_collection_renderer;
        $this->requestStack = $request_stack;
        $this->renderer = $renderer;
        $this->moduleHandler = $module_handler;
      }
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Sorry, you'll need to check it in 10.1.x

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    @mukesh88 any updates here? Anything I can help with - feel free to ping me on slack if you need more info

  • @larowlan Thanks for response. Actually i am busy on project i don't spent much time on contribution if i will free or sure will ping you on slack

  • First commit to issue fork.
  • Assigned to akshaydalvi212
  • 🇮🇳India akshaydalvi212

    I will resolve all the reported phpcs issues.

  • @akshaydalvi212 opened merge request.
  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • 🇮🇳India akshaydalvi212

    removed all the reported issues with PHPCS, and created the MR for the same.
    kindly review.

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    @akshaydalvi212 that merge request contains lots of changes that aren't relevant to PHPCS.

    I've closed it. We have a patch above, with actionable items. Let's work on top of this patch, or use it as the basis for an MR.

  • Status changed to Needs work over 1 year ago
  • First commit to issue fork.
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update over 1 year ago
    38 pass, 18 fail
  • @bharath-kondeti opened merge request.
  • Status changed to Needs review over 1 year ago
  • Status changed to Needs work over 1 year ago
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Tests are failing here

  • Hii,
    patch applied cleanly for #26 but still there are some errors.

    \entity_print\modules\entity_print_views\src\Controller\ViewPrintController.php
    ----------------------------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------------------------------------
    1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
    ----------------------------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------------------------------------

    \entity_print\src\Asset\AssetCollector.php
    ----------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------------------
    1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
    ----------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------------------

    \entity_print\src\Controller\EntityPrintController.php
    ----------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------------------
    1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
    ----------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------------------

    \entity_print\src\Entity\PrintEngineStorageInterface.php
    ----------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------------------
    1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
    ----------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------------------

    \entity_print\src\EntityPrintPermissions.php
    ----------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------------------
    1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
    ----------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------------------

    \entity_print\src\Event\PrintEventBase.php
    ----------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------------------
    1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
    ----------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------------------

    \entity_print\src\EventSubscriber\PrintEngineExceptionSubscriber.php
    -----------------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    -----------------------------------------------------------------------------------------
    1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
    -----------------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    -----------------------------------------------------------------------------------------

    \entity_print\src\Form\SettingsForm.php
    ----------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------------------
    1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
    ----------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------------------

    \entity_print\src\Plugin\EntityPrint\PrintEngine\DomPdf.php
    ----------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------------------
    1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
    ----------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------------------

    \entity_print\src\Plugin\EntityPrintPluginManager.php
    ----------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------------------
    1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
    ----------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------------------

    \entity_print\src\Plugin\PrintEngineInterface.php
    ----------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------------------
    1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
    ----------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------------------

    \entity_print\src\PrintBuilder.php
    ----------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------------------
    1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
    ----------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------------------

    \entity_print\src\Renderer\RendererBase.php
    ----------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ----------------------------------------------------------------------------------
    1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
    ----------------------------------------------------------------------------------
    PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    ----------------------------------------------------------------------------------

  • Status changed to Needs review 10 months ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update 10 months ago
    23 pass, 26 fail
  • 🇮🇳India mohd sahzad

    I have fixed this issue in above attached patch

  • Status changed to Needs review 10 months ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update 10 months ago
    Patch Failed to Apply
  • 🇮🇳India viren18febS

    i am not able to apply patch #28 , so i have applied patch #26 and its applied.
    after this patch i have found some more issues so i have added a patch for the fixes. please review.

  • Status changed to Needs work 10 months ago
  • Hi, I am not able to apply patch #30 it is throwing error.

    sha@LAPTOP-6VFKUABC MINGW64 ~/entity_print (8.x-2.x)
    $ git apply issues-3352346-Fixed-error-reported-by-phpcs.patch
    error: patch failed: src/Asset/AssetCollector.php:5
    error: src/Asset/AssetCollector.php: patch does not apply
    error: patch failed: src/Plugin/EntityPrint/PrintEngine/DomPdf.php:2
    error: src/Plugin/EntityPrint/PrintEngine/DomPdf.php: patch does not apply
    

    So changing the status to Needs work.
    Thankyou.

  • 🇮🇳India Yashaswi18

    Hello, I applied the patch for the version 8.x-2.x, the patch applied succesfully.

    Checking patch modules/entity_print_views/src/Controller/ViewPrintController.php...
    Checking patch src/Asset/AssetCollector.php...
    Checking patch src/Controller/EntityPrintController.php...
    Checking patch src/Entity/PrintEngineStorageInterface.php...
    Checking patch src/EntityPrintPermissions.php...
    Checking patch src/Event/PrintEventBase.php...
    Checking patch src/EventSubscriber/PrintEngineExceptionSubscriber.php...
    Checking patch src/Form/SettingsForm.php...
    Checking patch src/Plugin/EntityPrint/PrintEngine/DomPdf.php...
    Checking patch src/Plugin/EntityPrintPluginManager.php...
    Checking patch src/Plugin/PrintEngineInterface.php...
    Checking patch src/PrintBuilder.php...
    Checking patch src/Renderer/RendererBase.php...
    Applied patch modules/entity_print_views/src/Controller/ViewPrintController.php cleanly.
    Applied patch src/Asset/AssetCollector.php cleanly.
    Applied patch src/Controller/EntityPrintController.php cleanly.
    Applied patch src/Entity/PrintEngineStorageInterface.php cleanly.
    Applied patch src/EntityPrintPermissions.php cleanly.
    Applied patch src/Event/PrintEventBase.php cleanly.
    Applied patch src/EventSubscriber/PrintEngineExceptionSubscriber.php cleanly.
    Applied patch src/Form/SettingsForm.php cleanly.
    Applied patch src/Plugin/EntityPrint/PrintEngine/DomPdf.php cleanly.
    Applied patch src/Plugin/EntityPrintPluginManager.php cleanly.
    Applied patch src/Plugin/PrintEngineInterface.php cleanly.
    Applied patch src/PrintBuilder.php cleanly.
    Applied patch src/Renderer/RendererBase.php cleanly.

    Ran the command phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml entity_print/ after applying patch, there are no phpcs errors left.

  • Status changed to Needs review 10 months ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update 10 months ago
    Patch Failed to Apply
  • Status changed to Needs work 5 months ago
  • 🇺🇸United States smustgrave

    If the maintainers want to keep this type of issue then probably should be rescoped to cover the warnings from gitlab.

    Personal recommendation would be to close this out and start a new one since these type of tickets come off as farming.

Production build 0.71.5 2024