GitlabCI: Fix PHPCS validation errors

Like I said in 📌 Drupal best practices and coding standards Needs work :

Coding standard improvements must be provided as merge requests now, so that we can verify it using GitlabCI.

Additionally, this is way too big as a single patch and overlaps with many other issues. This is impossible to review and needs to be split up into issues for specific changes or groups of related changes.

pray_12 → made their first commit to this issue’s fork.

Updated Issue Summary and also created a MR 53. Please review. Still minified css error is there.

FILE: ...me/administrator/Projects/contribution/token/css/token.treetable.theme.css
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 1 | WARNING | File appears to be minified and cannot be processed
--------------------------------------------------------------------------------

Nikolay Shapovalov → made their first commit to this issue’s fork.

As requested by @Berdir I will create several smaller MRs, not sure if each MR should have separate issue.
First one https://git.drupalcode.org/project/token/-/merge_requests/54 is ready for review.

MRs are ready for review:

• 54
• 55
• 56

MR 57 is ready for review.

Nikolay Shapovalov → changed the visibility of the branch 2774071-fix-coding-standard to hidden.

MRs are ready for review:

• MR 54
• MR 55
• MR 56
• MR 57
• MR 58

Nikolay Shapovalov → changed the visibility of the branch 2774071-fix-comments to hidden.

Nikolay Shapovalov → changed the visibility of the branch 2774071-fix-comments to active.

I create branch 2774071-fix-cs-base-branch with phpcs.xml file and exclude all the rules that has any violation. Also gitlab ci should fail when there is phpcs found.

I rebased all existing MRs on this branch, and remove rules from phpcs that exists in this branch.

My suggestion to Merge base 2774071-fix-cs-base-branch to 8.x-1.x, in that case MR diff will be more transperent, right now it's impossible to see what rules will be fixed in the MR. But I add rules to MR description.

At the moment there are violations that still need to be fixed:

  <rule ref="Drupal">
    <exclude name="Drupal.Files.LineLength.TooLong"/>
    <exclude name="Drupal.NamingConventions.ValidVariableName.LowerCamelName"/>
    <exclude name="Drupal.Semantics.FunctionT.BackslashSingleQuote"/>
    <exclude name="Generic.PHP.UpperCaseConstant.Found"/>
    <exclude name="PSR2.Namespaces.UseDeclaration.SpaceAfterLastUse"/>
  </rule>

Created MR 65, now all MRs ready for review.

Not sure what to do with this.

Each of those MR's will conflict as they all change the phpcs and gitlab config.

I'd rather not change those files, including the allow fail. I'd rather rely on the code quality difference, which gitlabCI I think is still working on properly supporting. Then we can see if the situation gets worse or improves in a MR.

drupal.org issues are also not well suited to merge multiple merge requests per issue. We should either split them into separate issues or combine it.

DYdave → changed the visibility of the branch 2774071-fix-cs-base-branch to hidden.

DYdave → changed the visibility of the branch 2774071-visibility-array-indention to hidden.

DYdave → changed the visibility of the branch 2774071-comments-switch to hidden.

DYdave → changed the visibility of the branch 2774071-null-coalesce to hidden.

DYdave → changed the visibility of the branch 2774071-fix-cs-squiz to hidden.

DYdave → changed the visibility of the branch 2774071-use-fix-empty-line-fix to hidden.

DYdave → changed the visibility of the branch 2774071-left-violations to hidden.

DYdave → changed the visibility of the branch 2774071-fix-comments-debug1 to hidden.

Quick follow-up on this issue:

A few additional commits were added to the current merge request MR!58 with a few comments, see above at #58.

But mostly, at this point:
Last build on MR!58: https://git.drupalcode.org/issue/token-2774071/-/pipelines/181772

• The PHPCS job now seems to be passing ✅
  PHPCS job: https://git.drupalcode.org/issue/token-2774071/-/jobs/1684708
   
• The PHPUnit Tests are passing as well 🟢 \o/
  PHPUnit job: https://git.drupalcode.org/issue/token-2774071/-/jobs/1684713

These changes should be harmless enough, without any change in module's code (only doc comments) so they shouldn't take too long to review and would help enforcing validation of coding standards for other pending merge requests.

We would greatly appreciate if a maintainer or someone with write permission could take a look at ticket's merge request MR!58 and let us know if there would be any more work needed.

Feel free to let us know if you have any questions or concerns on merge request MR!58 or any aspect of this ticket in general, we would surely be glad to help.
Thanks in advance for your feedback and reviews.

Sorry about that: I got a bit confused here with the number of merge requests in this ticket and didn't really understand what had been done...

It appears the merge request that I modified above (#59 only fixes part of the issues : the ones related with the doc comments blocks mostly, see :
https://git.drupalcode.org/project/token/-/merge_requests/58/diffs#diff-...

As per #49 could someone please help creating some kind of "binder", with all the merge requests from this ticket merged into a single one, without a phpcs.xml file?
(without excluding any error from validation)

Maybe checking out each branch locally and merging it into a new one ("binder") would allow retaining every commit that was made for every merge request created in this issue.
There will mostly likely be some conflicts to be fixed as well when the various merge requests are combined.

Any help or feedback would be greatly appreciated.

Thanks in advance !