Fix the issues reported by phpcs

Added patch for the above phpcs fixes, please review it.

Thanks & Regards,
Mrinalini

Reviewed patch #3 patch applied successfully and all reported coding standard issue fixed. Looks good +1 RTBC

Hi Please check the MR: https://git.drupalcode.org/project/config_ignore/-/merge_requests/15

   /**
    * {@inheritdoc}
@@ -34,7 +31,9 @@ abstract class ConfigIgnoreBrowserTestBase extends BrowserTestBase {
   public function doImport() {
     // Set up the ConfigImporter object for testing.
     $storage_comparer = new StorageComparer(
-      $this->getImportStorage(),
+      $this->container->get('config.import_transformer')->transform(
+        $this->container->get('config.storage.sync')

Does a class that extends BroweserTestBase have access to $this->container? It is not listed in the class properties.

-//      'new config is not ignored in lenient mode' => [
-//        ['lenient'],
-//        ['config_test.*'],
-//        [
-//          '' => [
-//            'config_test.dynamic.exist' => ['id' => 'exist', 'label' => 'E'],
-//          ],
-//        ],
-//        [
-//          '' => [
-//            'config_test.dynamic.exist' => ['id' => 'exist', 'label' => 'N'],
-//            'config_test.dynamic.new' => ['id' => 'new', 'label' => 'N'],
-//            'config_test.system' => ['foo' => 'ignored']
-//          ],
-//        ],
-//        [
-//          '' => [
-//            'config_test.dynamic.exist' => ['id' => 'exist', 'label' => 'E'],
-//            'config_test.dynamic.new' => ['id' => 'new', 'label' => 'N'],
-//          ],
-//        ],
-//      ],
+      // 'new config is not ignored in lenient mode' => [
+      //        ['lenient'],
+      //        ['config_test.*'],
+      //        [

Line indentation, as per Drupal coding standards, is two spaces.
I am not sure it is worth editing commented out code, though.

-    // @todo: add meaningful tests in https://www.drupal.org/project/config_ignore/issues/2857247
+    // @todo add meaningful tests in https://www.drupal.org/project/config_ignore/issues/2857247

What follows @todo is a sentence: It starts with a capitalized word and ends with a period.
Also, that sentence is wrong: Tests are not added in an issue report; they are added as reported in an issue report.

adding patch, applied following phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig

-  - config_ignore
+  - drupal:config_ignore

The modules for this project are not part of Drupal core; their project machine name cannot be drupal.

 /**
- * Class ConfigIgnoreBrowserTestBase.
+ * Abstract class Definition.

That description does not describe the class. It does not even make sense, since that is not an abstract class.

-    $this->assertSame(['name' => 'config_ignore.settings', 'state' => 'Different'], $diff['config_ignore.settings']);
+    $this->assertSame([
+      'name' => 'config_ignore.settings',
+      'state' => 'Different',
+    ], $diff['config_ignore.settings']);

-        ['' => ['config_test.dynamic.exist' => ['id' => 'exist', 'label' => 'E']]],
+        [
+          '' => [
+            'config_test.dynamic.exist' =>
+        ['id' => 'exist', 'label' => 'E'],
+          ],
+        ],

Code lines can exceed 80 characters, if they are more readable. I do not find the changed code more readable.

-   * @param string $mode
-   *   The export mode
+   * @param string $modes
+   *   The export mode.

That method is declared as public function testExport(array $modes, array $patterns, array $active, array $sync, array $expected). It means that $modes type is array (not string) and its description must be The export modes. (since an array can contain more values).

    * @return array
+   *   Test case.

Since it is an array, its description should be The test cases.

-    // @todo: add meaningful tests in https://www.drupal.org/project/config_ignore/issues/2857247
+    // @todo add meaningful tests in https://www.drupal.org/project/config_ignore/issues/2857247

What follow @todo is a sentence: It starts with a capitalized word and it ends with a period (preferable), a question mark, or an exclamation point. If the sentences ends with a URL, the last punctuation mark is probably omitted.

Updated patch #17 by addressing #18, please review it.

Thanks!

Hi I reviewed the patch #19 .it applied cleanly. All the issues has been addressed and the update from #18 has been implemented. However the following errors is shown:

FILE: /home/arpitkayare/drupal_9.5/web/modules/contrib/config_ignore/tests/src/Kernel/IgnoreKernelTest.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------
166 | ERROR | The array declaration extends to column 82 (the limit is 80). The array content should be split up over multiple lines
166 | ERROR | The array declaration extends to column 81 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------------------------------------------------------------

FILE: /home/arpitkayare/drupal_9.5/web/modules/contrib/config_ignore/tests/src/Functional/ConfigWithTranslationTest.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
-------------------------------------------------------------------------------------------------------------------------------------
91 | ERROR | The array declaration extends to column 82 (the limit is 80). The array content should be split up over multiple lines
95 | ERROR | The array declaration extends to column 83 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------------------------------------------------------------

Time: 4.04 secs; Memory: 12MB

Thanks!

Let me work on this.

Fixed issues mentioned in #20. Please review.

Hi @sourabhjain I reviewed the patch #22. It applied cleanly all the errors/warnings are fixed. Running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,js . returned clean.
Keeping the status as Needs work as the test cases are failing that need to be be fixed as well.

Thanks!

One of the tests failed because the following error.

Error: Call to undefined method Drupal\Tests\config_ignore\Functional\ConfigIgnoreTest::t()

Replacing t() calls with calls to $this->t() is not sufficient.

Addressed failed test cases.

Updated the patch as the previous one was failing.

phpcs is now fixed on the main branch as part of the refactoring.
Thanks for working on this issue!

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

bircher → closed merge request !15

bircher → closed merge request !14