Fix the issues reported by phpcs

fixing the phpcs issues, there is one file which is still having some phpcs issues, but i think it is ok if we leave it as it is.

Deleted

I am reviewing the patch

while applying the patch got error

attaching the patch resolved the all issue please review

Reviewed patch #3 Patch not applying.

--   // Remove the page title of the user login page.
-+    // Remove the page title of the user login page.
-     if ($route = $collection->get('user.login')) {
-       $config = \Drupal::config('super_login.settings');
-       /*
--       $route->setDefaults(array(
--        '_title' => '',
--        '_form' => '\Drupal\user\Form\UserLoginForm',
-+      $route->setDefaults(array(
-+      '_title' => '',
-+      '_form' => '\Drupal\user\Form\UserLoginForm',

-     // Remove the page title of the password reset page.
-     if ($route = $collection->get('user.pass')) {
--       $config = \Drupal::config('super_login.settings');
-+      $config = \Drupal::config('super_login.settings');
-       /*
--       $route->setDefaults(array(
--        '_title' => '',
--        '_form' => '\Drupal\user\Form\UserPasswordForm',
-+      $route->setDefaults(array(
-+      '_title' => '',
-+      '_form' => '\Drupal\user\Form\UserPasswordForm',
-       ));
-        *
-        */
-     }
-   }
-+

A patch does not use -- nor -+. It is either + or -. That is why it does not apply.

I will work on this issue.

Resolved all the phpcs issues reported above.
providing new patch file will interdiff.
kindly review.

+  /**
+   * Config service.
+   *
+   * @var \Drupal\Core\Config\ConfigFactoryInterface

The short description is missing an article.

+  /**
+   * Constructor for creating service instance.
+   *
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
+   *   The config service.
+   */
+  public function __construct(ConfigFactoryInterface $configFactory) {

The short description for a class constructor is, for example, Constructs a \Drupal\views\Plugin\Block\ViewsBlockBase object. The class name must be indeed the correct one.

-      $config = \Drupal::config('super_login.settings');
+      // $config = \Drupal::config('super_login.settings');
+      $config = $this->configFactory->get('super_login.settings');

-       $config = \Drupal::config('super_login.settings');
+      // $config = \Drupal::config('super_login.settings');
+      $config = $this->configFactory->get('super_login.settings');

The existing code, if wrong, is replaced, not commented out.

 /**
- * @file 
+ * @file
  * Functions related to altering of login form.
  */

The short description for a module is Hook implementation for the [module name] module.

-    _form: '\Drupal\super_login\Super_loginSettingsForm'
+    _form: '\Drupal\super_login\SuperLoginSettingsForm'
     _title: 'Super Login Configuration'

The \Drupal\super_login\SuperLoginSettingsForm class does not exist.

hey @apaderno,
I will provide an updated patch file and a new patch and interdiff file.

provided the new patch and interdiff file.
kindly review.

+  /**
+   * Constructs a \Drupal\Core\Config\ConfigFactoryInterface object.
+   *
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
+   *   The config service.

That is not the correct class.

       /*
-       $route->setDefaults(array(
-        '_title' => '',
-        '_form' => '\Drupal\user\Form\UserLoginForm',
+      $route->setDefaults(array(
+      '_title' => '',
+      '_form' => '\Drupal\user\Form\UserLoginForm',
       ));
        *
        */

Since that is commented out code, it should be indented two spaces more respect the starting comment delimiter.

The src/Super_loginSettingsForm.php file cannot be simply removed. I do not see any reason to remove it, though, since it contains a class used from the module.

 /**
- * @file 
- * Functions related to altering of login form.
+ * @file
+ * Hook implementation for the super_login module.
  */

It is Hook implementations (plural). The module name is not its machine name. The module name is the one reported in its .info.yml file.

 super_login.settings:
   path: '/admin/config/people/super_login/settings'
   defaults:
-    _form: '\Drupal\super_login\Super_loginSettingsForm'
+    _form: '\Drupal\super_login\SuperLoginSettingsForm'

The \Drupal\super_login\SuperLoginSettingsForm class does not exist.

hey @apaderno,

the class name Super_loginSettingsForm is renamed to SuperLoginSettingsForm
as "the Class name must use UpperCamel naming without underscores" as per the standards.

and

can you please suggest some better short doc comment for the constructor class?
Thanks for the feedback.

the class name Super_loginSettingsForm is renamed to SuperLoginSettingsForm

No, the class name has not been renamed. Changing from _form: '\Drupal\super_login\Super_loginSettingsForm' to _form: '\Drupal\super_login\SuperLoginSettingsForm' is not renaming a class.

The short description for the constructor method is similar to Constructs a \Drupal\system\ConfigFormBase object. where \Drupal\system\ConfigFormBase is indeed replaced with the class of the built instance, not the class of any property.

Hi @akshaydalvi212 the patch #19 provided is not working couldn't be applied. Please see the attached screenshot with errors. Tested on Drupal 9.5.8 and super_login 2.0.X-dev.

Thanks!

Here is the updated patch. I am not able to raise a MR for some reason. so created a patch. One issue need to be taken care to update the Super_loginSettingsForm as the filename also need to be renamed to remove the underscore. reset issues has been taken care. The patch #19 couldnot be applied as my previous review. Please review the updated patch.

Thanks!

Hi, reviewed patch #24, but it couldn't be applied. Attached screenshot for reference.

Tried applying patch provided in #24, got this error:

git apply 3335836-24.patch -v
Checking patch src/Routing/RouteSubscriber.php...
error: while searching for:

namespace Drupal\super_login\Routing;

use Drupal\Core\Routing\RouteSubscriberBase;
use Symfony\Component\Routing\RouteCollection;


error: patch failed: src/Routing/RouteSubscriber.php:2
error: src/Routing/RouteSubscriber.php: patch does not apply
Checking patch src/Super_loginSettingsForm.php...
error: src/Super_loginSettingsForm.php: No such file or directory
Checking patch super_login.info.yml...
error: while searching for:
type: module
core: 8.x
core_version_requirement: ^8 || ^9
configure: super_login.settings
error: patch failed: super_login.info.yml:4
error: super_login.info.yml: patch does not apply
Checking patch super_login.module...
error: while searching for:
 * Functions related to altering of login form.
 */

use Drupal\Core\Url;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Link;


error: patch failed: super_login.module:5
error: super_login.module: patch does not apply
Checking patch super_login.services.yml...
error: while searching for:
    class: Drupal\super_login\Routing\RouteSubscriber
    tags:
      - { name: event_subscriber }

error: patch failed: super_login.services.yml:3
error: super_login.services.yml: patch does not apply

The report shown in the issue summary is for a module whose machine name is responsive_image. This project does not use that machine name.

Running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig shows the following warnings/errors which should be fixed.

FILE: ./config/install/super_login.settings.yml
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 14 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./config/schema/super_login.schema.yml
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 61 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./css/super-login.css
----------------------------------------------------------------------
FOUND 11 ERRORS AFFECTING 11 LINES
----------------------------------------------------------------------
   1 | ERROR | [x] Expected 1 space before opening brace of class
     |       |     definition; 0 found
   2 | ERROR | [x] Expected 1 space after colon in style definition;
     |       |     0 found
   5 | ERROR | [x] Expected 1 space before opening brace of class
     |       |     definition; 0 found
   6 | ERROR | [x] Expected 1 space after colon in style definition;
     |       |     0 found
   9 | ERROR | [x] Expected 1 space after colon in style definition;
     |       |     0 found
  10 | ERROR | [x] Expected 1 space after colon in style definition;
     |       |     0 found
  59 | ERROR | [x] Blank lines are not allowed in class definitions
 106 | ERROR | [x] Expected 1 space after colon in style definition;
     |       |     0 found
 130 | ERROR | [x] Expected 1 space before opening brace of class
     |       |     definition; 0 found
 131 | ERROR | [x] Expected 1 space after colon in style definition;
     |       |     0 found
 154 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 11 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./src/Routing/RouteSubscriber.php
----------------------------------------------------------------------
FOUND 10 ERRORS AND 6 WARNINGS AFFECTING 13 LINES
----------------------------------------------------------------------
 18 | ERROR   | [x] Line indented incorrectly; expected 4 spaces,
    |         |     found 3
 19 | WARNING | [ ] Unused variable $route.
 20 | WARNING | [ ] Unused variable $config.
 20 | WARNING | [ ] \Drupal calls should be avoided in classes, use
    |         |     dependency injection instead
 22 | ERROR   | [x] Line indented incorrectly; expected 6 spaces,
    |         |     found 7
 23 | ERROR   | [x] Line indented incorrectly; expected 6 spaces,
    |         |     found 8
 24 | ERROR   | [x] Line indented incorrectly; expected 6 spaces,
    |         |     found 8
 31 | WARNING | [ ] Unused variable $route.
 32 | WARNING | [ ] Unused variable $config.
 32 | ERROR   | [x] Line indented incorrectly; expected 6 spaces,
    |         |     found 7
 32 | WARNING | [ ] \Drupal calls should be avoided in classes, use
    |         |     dependency injection instead
 34 | ERROR   | [x] Line indented incorrectly; expected 6 spaces,
    |         |     found 7
 35 | ERROR   | [x] Line indented incorrectly; expected 6 spaces,
    |         |     found 8
 36 | ERROR   | [x] Line indented incorrectly; expected 6 spaces,
    |         |     found 8
 41 | ERROR   | [x] Expected 1 blank line after function; 0 found
 42 | ERROR   | [x] The closing brace for the class must have an
    |         |     empty line before it
----------------------------------------------------------------------
PHPCBF CAN FIX THE 10 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./src/Super_loginSettingsForm.php
----------------------------------------------------------------------
FOUND 7 ERRORS AND 9 WARNINGS AFFECTING 11 LINES
----------------------------------------------------------------------
   7 | WARNING | [x] Unused use statement
   7 | ERROR   | [x] Use statements should be sorted alphabetically.
     |         |     The first wrong one is
     |         |     Drupal\Core\Extension\ModuleExtensionList.
  12 | ERROR   | [ ] Class name doesn't match filename; expected
     |         |     "class Super_loginSettingsForm"
  12 | ERROR   | [ ] Class name must begin with a capital letter
  12 | ERROR   | [ ] Class name must use UpperCamel naming without
     |         |     underscores
  38 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
  92 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
  96 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
  97 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
  98 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 133 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 206 | ERROR   | [x] Functions must not contain multiple empty lines
     |         |     in a row; found 2 empty lines
 211 | WARNING | [ ] Unused variable $module_data.
 211 | WARNING | [ ] \Drupal calls should be avoided in classes, use
     |         |     dependency injection instead
 213 | ERROR   | [x] Expected 1 newline at end of file; 0 found
 213 | ERROR   | [x] The closing brace for the class must have an
     |         |     empty line before it
----------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./super_login.info.yml
----------------------------------------------------------------------
FOUND 1 ERROR AND 3 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
  1 | WARNING | [ ] Remove "project" from the info file, it will be
    |         |     added by drupal.org packaging automatically
  1 | WARNING | [ ] Remove "datestamp" from the info file, it will be
    |         |     added by drupal.org packaging automatically
  1 | WARNING | [ ] Remove "version" from the info file, it will be
    |         |     added by drupal.org packaging automatically
 10 | ERROR   | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./super_login.libraries.yml
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 21 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./super_login.module
----------------------------------------------------------------------
FOUND 13 ERRORS AND 10 WARNINGS AFFECTING 21 LINES
----------------------------------------------------------------------
   1 | ERROR   | [x] Missing file doc comment
   3 | WARNING | [x] Unused use statement
   4 | ERROR   | [x] Use statements should be sorted alphabetically.
     |         |     The first wrong one is
     |         |     Drupal\Core\Form\FormStateInterface.
   8 | ERROR   | [x] Whitespace found at end of line
  13 | WARNING | [ ] Format should be "* Implements hook_foo().", "*
     |         |     Implements hook_foo_BAR_ID_bar() for
     |         |     xyz_bar().",, "* Implements
     |         |     hook_foo_BAR_ID_bar() for xyz-bar.html.twig.",
     |         |     "* Implements hook_foo_BAR_ID_bar() for
     |         |     xyz-bar.tpl.php.", or "* Implements
     |         |     hook_foo_BAR_ID_bar() for block templates."
  13 | ERROR   | [x] Whitespace found at end of line
  14 | ERROR   | [ ] Doc comment short description must end with a
     |         |     full stop
  14 | ERROR   | [ ] Doc comment short description must be on a
     |         |     single line, further text should be a separate
     |         |     paragraph
  28 | WARNING | [ ] Only string literals should be passed to t()
     |         |     where possible
  30 | ERROR   | [x] Array indentation error, expected 6 spaces but
     |         |     found 8
  31 | ERROR   | [x] Array indentation error, expected 6 spaces but
     |         |     found 8
  37 | ERROR   | [x] Functions must not contain multiple empty lines
     |         |     in a row; found 2 empty lines
  43 | ERROR   | [x] Functions must not contain multiple empty lines
     |         |     in a row; found 2 empty lines
  62 | WARNING | [ ] Only string literals should be passed to t()
     |         |     where possible
  66 | WARNING | [ ] Only string literals should be passed to t()
     |         |     where possible
  74 | WARNING | [ ] Only string literals should be passed to t()
     |         |     where possible
  81 | ERROR   | [x] Namespaced classes/interfaces/traits should be
     |         |     referenced with use statements
  84 | WARNING | [ ] Only string literals should be passed to t()
     |         |     where possible
  90 | WARNING | [ ] Only string literals should be passed to t()
     |         |     where possible
 125 | WARNING | [ ] Only string literals should be passed to t()
     |         |     where possible
 129 | WARNING | [ ] Only string literals should be passed to t()
     |         |     where possible
 171 | ERROR   | [x] Expected 1 space after closing parenthesis;
     |         |     found 0
 185 | ERROR   | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 12 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./super_login.services.yml
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 5 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 139ms; Memory: 10MB

I didn't change the Super_loginSettingsForm class name, which I would rather change in a different issue, since that is not a BC change.
I still have to remove a call to a \Drupal method in a class.

@apaderno - I'm new to working with merge requests and this is my first one. I merged it on drupalcode.org and then went here to the issue and selected to merge it but I simply get "merged failed" with no additional information (see screenshot). Any idea where the issue may be?