Fix code style/quality issues

Created on 20 April 2019, over 5 years ago
Updated 26 December 2023, 11 months ago

Now that we are getting closer to an MVP, we need to start paying more attention to code quality. We are running phpcs, phpmd and PHPMetrics, but because of the huge number of issues we cannot reliably enable this on our CI runs. Let's fix the existing issues and be more strict about code style and quality going forward:

PHPCS issues:
https://281-167400708-gh.circle-artifacts.com/0/var/www/html/artifacts/p...

This is the most important one to look at!

PHPMD issues:
https://281-167400708-gh.circle-artifacts.com/0/var/www/html/artifacts/p...

These are less important, and the current configuration might be too strict! Think about adding a phpmd.xml configuration file to the root of the module.

🐛 Bug report
Status

Needs work

Version

1.0

Component

Code

Created by

🇺🇸United States Etroid

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.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇮🇳India arpitk

    Looking into it.

  • Merge request !103049711-fix code style/quality issues. → (Open) created by arpitk
  • Issue was unassigned.
  • 🇮🇳India arpitk

    Worked on major issues reported by phpcs.

  • First commit to issue fork.
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    75 pass, 2 fail
  • 🇳🇱Netherlands megachriz

    I've rebased the code on the latest dev, actually just so that the tests run.

  • 🇳🇱Netherlands megachriz

    @arpitk
    Thanks for your work, but it looks like that some of the changes break tests.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    82 pass
  • 🇳🇱Netherlands megachriz

    I made some small changes and now tests are passing again. :)

    Still needs work though, because there are more coding standards violations.

  • Assigned to nitin_lama
  • Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update 11 months ago
    Not currently mergeable.
  • 🇮🇳India nitin_lama India

    Remaining issues.

    FILE: /home/system/Documents/contribution/feeds_migrate-3049711/feeds_migrate.module
    ------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
    ------------------------------------------------------------------------------------
      68 | WARNING | Line exceeds 80 characters; contains 83 characters
      88 | WARNING | Line exceeds 80 characters; contains 82 characters
     116 | WARNING | Line exceeds 80 characters; contains 82 characters
    ------------------------------------------------------------------------------------
    
    
    FILE: /home/system/Documents/contribution/feeds_migrate-3049711/src/FeedsMigrateBatchExecutable.php
    -------------------------------------------------------------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    -------------------------------------------------------------------------------------------------------------------------------------
     43 | ERROR | The array declaration extends to column 96 (the limit is 80). The array content should be split up over multiple lines
    -------------------------------------------------------------------------------------------------------------------------------------
    
    
    FILE: /home/system/Documents/contribution/feeds_migrate-3049711/src/Plugin/MigrateFormPluginInterface.php
    -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
    FOUND 2 ERRORS AFFECTING 1 LINE
    -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
     56 | ERROR | The text '@deprecated This will be removed in favor of ::getConfiguration. Plugins should not save its configuration directly on the parent entity.' does not match the standard format:
        |       | @deprecated in %deprecation-version% and is removed from %removal-version%. %extra-info%.
     56 | ERROR | Each @deprecated tag must have a @see tag immediately following it
    -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
    
    Time: 706ms; Memory: 12MB
    
    FILE: /home/system/Documents/contribution/feeds_migrate-3049711/src/MappingFieldFormManager.php
    -----------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    -----------------------------------------------------------------------------------------------
     77 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
    -----------------------------------------------------------------------------------------------
    
    
    FILE: /home/system/Documents/contribution/feeds_migrate-3049711/src/Form/FeedsMigrateImporterForm.php
    -----------------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    -----------------------------------------------------------------------------------------------------
     541 | WARNING | Unused variable $status.
    -----------------------------------------------------------------------------------------------------
    
    
    FILE: /home/system/Documents/contribution/feeds_migrate-3049711/src/Plugin/MigrateFormPluginFactory.php
    -------------------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    -------------------------------------------------------------------------------------------------------
     85 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
    -------------------------------------------------------------------------------------------------------
    
    
    FILE: /home/system/Documents/contribution/feeds_migrate-3049711/src/Plugin/feeds_migrate/mapping_field/TextAreaFieldForm.php
    ----------------------------------------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    ----------------------------------------------------------------------------------------------------------------------------
     27 | WARNING | Unused variable $test.
    ----------------------------------------------------------------------------------------------------------------------------
    
    
    FILE: /home/system/Documents/contribution/feeds_migrate-3049711/src/Plugin/MigrateFormPluginManager.php
    -------------------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    -------------------------------------------------------------------------------------------------------
     81 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
    -------------------------------------------------------------------------------------------------------
    
    
    FILE: /home/system/Documents/contribution/feeds_migrate-3049711/tests/src/Functional/FeedsMigrateBrowserTestBase.php
    -------------------------------------------------------------------------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    -------------------------------------------------------------------------------------------------------------------------------------------------
     52 | ERROR | Do not disable strict config schema checking in tests. Instead ensure your module properly declares its schema for configurations.
    -------------------------------------------------------------------------------------------------------------------------------------------------
    
    Time: 646ms; Memory: 12MB
    
Production build 0.71.5 2024