Side effects

Minimum Drupal Core version has been upped to 10.3, as a result of the changes made.

Problem/Motivation

The module does not fully comply with Drupal coding standards.
Some comments, annotations, and/or code formatting do not follow the
conventions defined by Drupal Coding Standards
.

This may reduce code readability, hinder automated checks such as
PHP_CodeSniffer (PHPCS), and affect long-term maintainability.

Steps to reproduce

Download the module.

Run phpcs --standard=Drupal,DrupalPractice modules/custom/[module_name].

Review the reported errors and warnings.

Proposed resolution

Update the code to comply with Drupal coding standards:

Adjust comments and function/class descriptions to 80 characters.

Add @see tags to deprecated functions when required.

Fix spacing, indentation, and curly braces placement.

Use recommended Drupal naming conventions and annotations.

Remaining tasks

Identify all PHPCS issues within the module.

Fix the code according to the reports.

Re-run phpcs to confirm no remaining errors.

Submit a merge request with the changes applied.

Issue fork samlauth-3557441

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

andresalvarez created an issue. See original summary.

andresalvarez opened merge request !39

andres alvarez’s picture

andres alvarez commented
Status: Active » Needs review

roderik made their first commit to this issue’s fork.

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
Issue summary: View changes
Status: Needs review » Fixed

If this module was more actively maintained, I would have sent this back earlier for more work, and not committed it. (Then again, if this module was more actively maintained, this MR likely wouldn't exist.)

This contains useful things and fixes for the gitlab pipeline; some of the code needed fixing since a long time. It also contains changes to translations, comments and README that are IMHO nonsensical or unnecessary; type additions that could cause incompatibility with child classes (so I want to delay those until a next major version until they cause issues in e.g. tests/CI); blunt removals of 'todo' code/comments, and code changes that should not be mixed in with this PR and not applied without explanation. (Why replace $_POST with getRequest() without any comment?)

It is, in other words, a mess that required work to untangle, which is the reason it took longer. This is generally not how to submit MRs to an open source project. I might have reverted some improvements during cleanup; if you want to have those missed improvements committed, please open another MR - and add comments if they are not trivial.

I've also re-done a few things differently, because I needed to mess with it anyway. I would comment on the reasons, if I knew that was useful.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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