Fix the issues reported by PHP_CodeSniffer

The last submitted patch, 20: 3291954-20.patch, failed testing. View results →

More fixes of PHPCS issues.

Patch #26 failed to apply. There are several issues remaining

urvasi@urvasi-Inspiron-15-3552:/var/www/html/contribution/drupal/web/modules/contrib$ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,js,info,txt,md,yml,twig form_mode_manager-3291954/

FILE: ...rib/form_mode_manager-3291954/tests/src/Functional/FormModeManagerBase.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
--------------------------------------------------------------------------------
 171 | WARNING | t() calls should be avoided in classes, use
     |         | \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         | $this->t() instead
 180 | WARNING | t() calls should be avoided in classes, use
     |         | \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         | $this->t() instead
--------------------------------------------------------------------------------


FILE: ...b/form_mode_manager-3291954/tests/src/Functional/FormModeManagerUiTest.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
--------------------------------------------------------------------------------
 256 | WARNING | t() calls should be avoided in classes, use
     |         | \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         | $this->t() instead
 339 | WARNING | t() calls should be avoided in classes, use
     |         | \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         | $this->t() instead
 359 | WARNING | t() calls should be avoided in classes, use
     |         | \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         | $this->t() instead
--------------------------------------------------------------------------------


FILE: ...orm_mode_manager-3291954/tests/src/Functional/FormModeManagerRouteTest.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
--------------------------------------------------------------------------------
  29 | WARNING | Unused variable $node_type_test_page.
 238 | WARNING | t() calls should be avoided in classes, use
     |         | \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         | $this->t() instead
--------------------------------------------------------------------------------


FILE: ...les/contrib/form_mode_manager-3291954/src/Form/FormModeManagerFormBase.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 139 | WARNING | t() calls should be avoided in classes, use
     |         | \Drupal\Core\StringTranslation\StringTranslationTrait and
     |         | $this->t() instead
--------------------------------------------------------------------------------

Time: 3.71 secs; Memory: 12MB

I will work on them. Assigning it to myself. Thanks

Fixed all remaining issues. Committing the changes. Please review.

Hi I rewieved all the changes

I ran these commands phpcs --standard=DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml

PHPCS errors has been fixed.
Rebase already have done
Moving to RTBC
(Tests is failing before the issue)

(Tests is failing before the issue)

Not true. Tests are failing like so:

Error: Call to undefined method Drupal\Tests\form_mode_manager\Functional\FormModeManagerUiTest::t()

That's because of these kinds of changes in the MR:

      ->pageTextContains(t('The configuration options have been saved.'));
      ->pageTextContains($this->t('The configuration options have been saved.'));

This is completely the wrong "fix" for these. We shouldn't use t() at all in tests (unless we're explicitly testing translatability). See 📌 [meta] Remove usage of t() in tests not testing translation Active .

For all the changes like the above, we really want:

      ->pageTextContains('The configuration options have been saved.');

This is part of why I've come to completely hate these "fix phpcs" issues that everyone seems so excited about trying to get credit for. Y'all use automated tools to get some changes done, then make a mess of things you don't fully understand, then lots of folks pile on to "review" and "validate" the brokenness. The whole thing is a giant waste of time for almost no value. 😢

I'm tempted to close this as "won't fix"...

Hi I fixed the tests that you mentioned before, but I checked the 8.x-1.x version of this module there has a lot of failing tests like this

Testing Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest
EREEEEEEEEEEEEEEEEE                                               19 / 19 (100%)E

Time: 01:05.083, Memory: 12.00 MB

There were 19 errors:

1) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testAnonymousSpecificFormModeManagerRoutes
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

2) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testAddFormModeManagerRoutes
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

3) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testEditFormModeManagerRoutes
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

4) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testUserEditFormModeManagerRoutes
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

5) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testListWithOneFormModeManagerRoutes
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

6) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testListWithTwoFormModeManagerRoutes
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

7) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testFormModeManagerPlugin with data set #0 ('node', 'add_form', 'node.add')
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

8) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testFormModeManagerPlugin with data set #1 ('node', 'edit_form', 'entity.node.edit_form')
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

9) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testFormModeManagerPlugin with data set #2 ('user', 'add_form', 'user.register')
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

10) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testFormModeManagerPlugin with data set #3 ('user', 'edit_form', 'entity.user.edit_form')
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

11) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testFormModeManagerPlugin with data set #4 ('user', 'admin_add', 'user.admin_create')
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

12) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testFormModeManagerPlugin with data set #5 ('block_content', 'add_form', 'block_content.add_form')
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

13) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testFormModeManagerPlugin with data set #6 ('block_content', 'edit_form', 'entity.block_content.edit_form')
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

14) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testFormModeManagerPlugin with data set #7 ('taxonomy_term', 'add_form', 'entity.taxonomy_term.add_form')
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

15) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testFormModeManagerPlugin with data set #8 ('taxonomy_term', 'edit_form', 'entity.taxonomy_term.edit_form')
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

16) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testFormModeManagerPlugin with data set #9 ('media', 'add_form', 'entity.media.add_form')
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

17) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testFormModeManagerPlugin with data set #10 ('media', 'edit_form', 'entity.media.edit_form')
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

18) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testAdminRoutes with data set #0 ('form_mode_manager.admin_settings')
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

19) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testAdminRoutes with data set #1 ('form_mode_manager.admin_setti...s_task')
Exception: Drupal\Tests\BrowserTestBase::$defaultTheme is required. See https://www.drupal.org/node/3083055, which includes recommendations on which theme to use.

/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:436
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:544
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:364
/app/web/modules/contrib/form_mode_manager/tests/src/Functional/FormModeManagerBase.php:94
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

--

There was 1 risky test:

1) Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::testAnonymousSpecificFormModeManagerRoutes
This test did not perform any assertions

/app/web/core/tests/Drupal/Tests/Listeners/DrupalListener.php:127
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:452
/app/vendor/phpunit/phpunit/src/Util/PHP/AbstractPhpProcess.php:377
/app/vendor/phpunit/phpunit/src/Util/PHP/AbstractPhpProcess.php:187
/app/vendor/phpunit/phpunit/src/Framework/TestSuite.php:673
/app/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:661
/app/vendor/phpunit/phpunit/src/TextUI/Command.php:144
/app/vendor/phpunit/phpunit/src/TextUI/Command.php:97

ERRORS!
Tests: 19, Assertions: 0, Errors: 19, Risky: 1.

Remaining self deprecation notices (19)

  19x: The Drupal\Tests\form_mode_manager\Functional\FormModeManagerRouteTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    19x in DrupalListener::startTest from Drupal\Tests\Listeners

My suggestion is opening another issue to fix all the tests in 8.x-1.x branch

Hi please revise.

Hi @Rakhi,

Confirmed fixed the PHPCS errors after applying plain diff file to the Form mode manager module against the 8.x-2.x-dev and with the Drupal core of 9.5.6.

Moving this now to RTBC.
Kindly refer to the screenshots attached, please.

Thank you.

 /**
- * Class FormModeThemeNegociator.
+ * {@inheritDoc}
  */

{@inheritDoc} is not used for a class documentation comment.

 /**
- * Interface EntityFormModeManagerInterface.
+ * {@inheritDoc}
  */
 interface EntityFormModeManagerInterface {

It is not even used for an interface documentation comment.

- * with form mode manager and his form modes associated.
+ * with form mode manager and it's form modes associated.

The correct word is its not it's, since that is the possessive for the third person it.

 /**
- * Interface FormModeManagerInterface.
+ * Interface For FormModeManagerInterface.
  */
 interface FormModeManagerInterface {

That is not the correct short description for an interface. It does not even make sense, since an interface is not an interface for itself.

 /**
- * Class BlockContent.
+ * Class For BlockContent.
  *
  * @EntityRoutingMap(

A class short description must not start with Class For, nor merely repeat the class name.

I will work on it.

Hi I just fixed the issues reported before. Please revise

Hi @elber,

After applying your MR !6 and rerunning the phpcs, a new warning was displayed.

Kindly refer to the screenshots attached, please.

Thanks.

Hi it was happening before my changes but I fixed it now, please revise.

Hi @elber,

Confirmed fixed now after applying your latest MR.

Moving this now to RTBC.
Please refer the attached screenshot.

Thanks.

 /**
- * Class FormModeThemeNegociator.
+ * Class to define theme negotiator options.
  */
 class FormModeThemeNegociator implements ThemeNegotiatorInterface {
 

/**
- * Interface EntityFormModeManagerInterface.
+ * Interface to manage form options.
  */

Class short descriptions do not start with Class.
Interface short descriptions do not start with Interface.

- * specific FormClass, but the operation name and routes linked with her are,
+ * specific FormClass, but the operation name and routes linked with it are,

It is form class.

 /**
- * Interface FormModeManagerInterface.
+ * Load the Entity form options.
  */

The verb must use the third person singular.
Entity is misspelled, since it is not the first word in the sentence.

   * entities show up immediately. This is wrapped by Form Mode Manager to,
    * permit a more precise cache strategy and allow Form Mode Manager to,
-   * add her permissions tags.
+   * add its permissions tags.

No comma is added between to and the following verb.

 /**
- * Class BlockContent.
+ * Defines the custom block entity class.

The verb is not necessary.

 /**
- * Class Node.
+ * Class For Node.

The previous change I quoted is correct. This change is not.

 /**
- * Class Term.
+ * {@inheritDoc}
  *

{@inheritDoc} is not used in class documentation comments.

  * In Entity API we have possibility to linked entity form 'handlers' to a,
- * specific FormClass, but the operation name and routes linked with her are,
+ * specific FormClass, but the operation name and routes linked with it are,

It is form class, not FormClass.

+ * specific FormClass, but the operation name and routes linked with it are,
  * very arbitrary and unpredictable specially in custom entities cases.

English does not put commas between a verb and its object. It is not My cat is, lovely. but My cat is lovely.

/**
- * Interface FormModeManagerInterface.
+ * Loads the entity form options.
  */
 interface FormModeManagerInterface {

I am not sure that is a suitable description for an interface, as an interface just defines a list of methods; it does not load entities.

 /**
- * Class Node.
+ * Create basic nodes.

Assuming that class really creates basic nodes, the verb must use the third person singular.

 /**
- * Class Term.
+ * Taxonomy term.

For a Term class, that description does not say much.

 /**
- * Class User.
+ * Class For User.

Adding For (which should not be capitalized) does not improve that documentation comment. It even makes the description wrong, since a User class is not a class for User.

To make the text were we had FormClass more redable, I changed a couple of things. Looks like this now:

This plugin are used to abstract the concepts implemented by EntityPlugin.
In Entity API we have the possibility to link entity form 'handlers' to a specific form class, but the operation name and routes linked with it are very arbitrary and unpredictable specially in custom entities cases.
In that plugin you have the possibility to map operation and others useful information about entity to reduce complexity of retrieving each possible cases.

The interface description I changed to "An interface to get and return information on form modes."

And for "class node", "class term" and "class user" I changed to "Creates ___ routes".

Are those better?
Thank you for reviewing.

Changed the comments based on the review, thank you!

Hi reviewed the changes.

PHPCS errors has been fixed I ran

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

Apaderno's suggestions were made

It sounds good to me.

moving to RTBC.

Hi,

Reviewed the latest MR and applied it to the module, confirmed no PHPCS errors were detected.

Please see the attached file.

Moving this to RTBC.

Thank you!

Updated code comments with proper English.
Added string translation back to tests.

To make the last point clearer: Tests do not normally set a language for the tested pages, which means that messages in those pages are always in English. It is useless to call $this->t() or t() when the returned message is the same string passed to that method/function.

The issue summary reports errors/warnings for 12 files, but the MR changes 18 files. Either the issue summary must be updated, or the MR is changing more files than necessary.

Oh bother. I merged all of the PHPCS changes created as a sub-merge of 📌 Automated Drupal 10 compatibility fixes RTBC into it. That was probably a mistake. It is going to be a mess to merge both together as it currently reports no phpcs output.

@ELC, it was a small mistake, however, it is easy enough to fix. I'll do it.

Oops introduced some whitespace.

Hi everyone. Thanks to everyone trying to help in here and improve the code. Sorry I let this drag on for so long. The level of noise being generated in here is very high...

@apaderno: While your desire to educate and train other contributors is very admirable, in this case, I'd recommend either:

1. Properly using GitLab suggestions so that it's easier to adopt your proposed changes.
2. Or maybe better yet, just push a commit to fix things. 😅 I wouldn't have cared that you both wrote and reviewed, and would have accepted an RTBC from you, regardless...

I tried to go through and save credit for everyone who seemed to be attempting to contribute here in good faith. A few "contributions" were borderline, apologies if you feel left out.

I rebased this after committing 📌 Remove t() calls in tests not testing translation Fixed , pushed a few other fixes, merged to 8.x-2.x, and pushed commit eadc8e36fc

Calling this fixed at last! 🎉

dww → closed merge request !6

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