@solideogloria

Shorter issue title

If you aren't sure whether a site uses the feature, uninstall the module and export site config in a dev environment. If a config file besides core.extension.yml was changed, then the module was still used.

solideogloria → created an issue.

From the module page:

Important updates in the 3.0.0 release (3.0.x branch)

• Since Drupal 10.1.0, limiting the text formats per field instance is a feature provided by Drupal core. Read https://www.drupal.org/node/3318572 → for details.
• In the 3.0.0 release, the allowed formats feature has been removed as obsolete, but we provide an update path from existing sites to move the allowed formats, as they were stored by the previous versions of the module, to Drupal >=10.1.0 way, in field settings.
• The module provides also a feature that allows site builders to hide the formatted text format help and guidelines. Even this feature is still preserved in the 3.0.x module branch, there is an issue that aims to move it in Drupal core in the future. See https://www.drupal.org/i/3323007 → .

So it depends if you use the feature of this module to hide the formatted text format help and guidelines. That functionality isn't in Core yet.

solideogloria → created an issue.

solideogloria → created an issue.

Is this still happening? I just tried it in Drupal 10.2, without using this module, and it doesn't reveal any information.

@Prashant.c That issue hasn't been finished yet, so the service doesn't exist.

Those things are already supported by the existing coding standards, as I already use them in projects. They just aren't documented very well.

[144538] was committed, which added a new UserLogoutConfirm form calling user_logout().

The MR will need to be rebased and then the form will need to be updated to use the new service.

In addition, there is a merge request open, so please refrain from making changes by submitting patches. It's hard to see what changed without an interdiff, and the pipeline won't run.

All issues should target the 11.x branch.

Minor typo fixes

An alternative to saving credentials in config is to use the Key → module, or to use settings that must be set in settings.php.

The module's composer.json file requires "drupal/smsframework": "^2.1",, however, that version of SMS Framework doesn't support Drupal 10. You would need to use "drupal/smsframework": "^2.2@RC",

It is stored in the plugin configuration

Are you sure? The class doesn't extend the typical configuration form base. There is no call to ->save() anywhere.

I noticed that there is a password field.

    $form['every8d']['password'] = [
      '#type' => 'textfield',
      '#title' => $this->t('Password'),
      '#default_value' => $config['password'],
      '#required' => TRUE,
    ];

The type should be password, not textfield.

Also, it's not clear to me where the password is stored, if anywhere. It doesn't appear to save it in config. If it did, you might want to encourage users to put the password in settings.php or use the Key → module.

I agree with #25, that we should use something more extensible. However, I don't think either of the suggestions is enough information for making it work. The issue is in the FormBuilder class, and at that point, you don't have instances of ElementInterface, you have a render array. So you can't do an instanceof test.

It might be easier to add an attribute #is_composite to the render array?

Or if I'm wrong, then I would say it's probably better to have CompositeFormElementInterface, because there is also a CompositeFormElementTrait. So you could even put isComposite in the trait, and it would be added to any contrib/custom classes using the trait as well.

Added stack trace

Consider using DDEV and DDEV Drupal Contrib for module development, as it makes running PHPCS and PHPStan super easy. Using it I can pull your project's code and run everything on it in under a minute. You can also run ddev phpcbf and have it auto-fix the mistakes.

Many of the PHPCS errors from above are still not fixed. Please set up and configure PHPCS properly.

vendor/bin/phpcs --standard=Drupal,DrupalPractice --ignore=vendor --extensions=php,inc,module,install,info,test,profile,theme --parallel=2 ./

Or use this phpcs.xml.dist:

<?xml version="1.0" encoding="UTF-8"

Default PHP CodeSniffer configuration for Drupal project.

?>

@DYdave Could you also mark the 2.0.x branch as a recommended branch, since it has a stable release and is covered by the security advisory policy?

Actually, PHPCS passed.

poker10 → credited solideogloria → .

solideogloria → created an issue.

Patch for convenience.

solideogloria → created an issue. See original summary → .

I think the changes should be applied to the MR, to avoid confusion. The issue has an open merge request, so changes should not have been made by submitting patches.

I'm not able to review this. I'm still stuck on rc5, due to 🐛 The new sidebar makes UI unusable when using Layout Builder Admin Theme Active .

@apaderno Then I think the PAReview.sh script should be updated. My comment about the LICENSE file is from that script's output.

@phjou Could you also look at the issues mentioned in #7 🐛 Sidebar dialog close button is not visible in some cases Needs review and see if you have them, too?

FILE: src/Plugin/Field/FieldFormatter/AddressStaticMapFormatter.php

50: Data types in @param tags need to be fully namespaced
180: \Drupal calls should be avoided in classes. Use dependency injection.

I think it would be a good idea to break out the code that is used to "Generate signature from premium crypto key or APY key signing secret" into its own function. The function its currently in is for rendering, which doesn't need to be "in charge of" signature generation.

"Must be between 1 and 16 (inclusive) for Mapquest"

MapQuest should have the "Q" capitalized. I don't see any validation to make sure the user doesn't select an invalid value for a MapQuest map. Also, the module states that MapQuest isn't supported yet, so maybe this message should be removed until it is?

The code looks good to me. There aren't a lot of commits on the dev branch for some reason. There are more commits on 1.0.3.

solideogloria → created an issue.

Plus, DDEV has the ddev-drupal-contrib addon, which makes it super easy to contribute to merge requests for contrib projects, and it allows you to easily run PHPCS and PHPUnit tests.

The file name was changed in the patch, so they actually do match. However, you cannot simply apply the patch with Composer, because Windows computers won't rename the file if the letters are the same, even if the case is different.

See my previous comments in #9. 📌 Fixes for Drupal coding standards Needs review

The code changes do not follow Drupal coding standards.

I will also note that this module is extremely small, having only 3 files (only one of them a PHP file), and I'm not sure if this is enough code to properly demonstrate someone's ability to follow coding standards and create secure code.

if ($this->getSetting('trim_summary') == TRUE && !empty($item->summary)) {

You can remove the == TRUE here.

Otherwise it looks pretty good to me.

Also, if you could remove the words "Summary Only" at the top of the module page on Drupal.org, I think it'd look cleaner:

Summary Only
This module provides a field formatter that helps to render only summary value
of any field having type "text_with_summary".

When you call $ids = $this->entityStorage->getQuery()->execute();, you should add a call to checkAccess

Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. See https://www.drupal.org/node/3201242 →

ESLint:

js/mosparo_integration.frontend.js
  11:87  error  Insert `,`  prettier/prettier
  34:48  error  Insert `,`  prettier/prettier
  40:8   error  Insert `,`  prettier/prettier

solideogloria → created an issue.

Your README.md does not follow best practices → .

INSTALL.md contains outdated instructions. Remove info about Drupal 7. The images don't load.

The files in ./config/optional have CRLF line terminators. These should be changed to Unix-style LF line terminators → .

languagewire_translation_provider.info.yml: All dependencies must be prefixed with the project name. Also, remove version and timestamp info. Those will be added automatically. See a good example in the Webform module.

There are multiple classes that use \Drupal calls. Use dependency injection instead, if possible.

The .module and .install files aren't even close to following Drupal coding standards. Sort your use statements alphabetically, have functions' opening braces on the same line as the function declaration. Include function comments for every function. TRUE, FALSE and NULL must be uppercase. Functions not in a class/trait/enum should be snake_case and prefixed with the module name.

Many of the issues can be automatically found by PHPCS and fixed by PHPCBF if you run those. Please run them → , because there are many more issues that I have not listed, as there are too many. You should also configure your IDE to integrate with PHPCS, so that it will highlight coding standards violations as you type.

Please update the module page → to have more readable formatting. Use headings and subheading. Look at Coder → as an example.

Also, in the future, please consider using semantic versioning → for branch names and releases.

I'm not able to view the project repository for some reason right now, as it says "Deploy in progress"...

I'm also a bit concerned that despite this module providing a way to run PHPCS and other code quality checking tools, this module's code doesn't seem to have been run by them and doesn't meet Drupal coding standards. That's why I recommend that the module maintainer spend some time getting GitLab integration set up and working to get the IDE PHPCS integration.

While we can provide a list of items to fix, if you're only fixing them based on what we put in the comments, that doesn't guarantee that issues won't be reintroduced in future commits, without getting found or fixed.

As you can see, there is quite a lot to work on. I recommend getting your dev environment and IDE set up with PHPCS, so that you see issues as you type.

I also would recommend adding a .gitlab-ci.yml file → , so that in the future every time you push changes they will be automatically validated.

$ ./pareview.sh https://git.drupalcode.org/project/testsuite 3.0.x

Review of the 3.0.x branch (commit 0227b7f):

• Your README.md does not follow best practices (headings need to be uppercase). See https://www.drupal.org/node/2181737 → .
  • The INTRODUCTION section is missing.
  • The CONFIGURATION section is missing.
• Remove LICENSE.txt, it will be added by drupal.org packaging automatically.
• The list_packages.module does not implement hook_help(). See https://www.drupal.org/docs/develop/documenting-your-project/module-docu... → .
• ./modules/list_packages/list_packages.admin.inc: All functions should be prefixed with your module/theme name (list_packages) to avoid name clashes. See https://www.drupal.org/node/318#naming →
  

  function list_libraries_filters() {
  function list_modules_filters() {
  function list_module_libraries_filters() {
  function list_core_libraries_filters() {
  function comparator($object1, $object2) {
  

• Bad line endings were found, always use unix style terminators. See https://www.drupal.org/coding-standards#indenting →
  

  ./css/filter_forms.css:                                                      ASCII text, with CRLF line terminators
  css/filter_forms.css
  

• Coder Sniffer → has found some issues with your code (please check the Drupal coding standards → ). See attachment.
• Stylelint has found some issues with your code (please check the CSS coding standards → ). See attachment.
• DrupalPractice → has found some issues with your code, but could be false positives.
  

  
  FILE: ...eview_temp/modules/list_packages/src/ListPackagesResourceService.php
  --------------------------------------------------------------------------
  FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
  --------------------------------------------------------------------------
   420 | WARNING | \Drupal calls should be avoided in classes, use
       |         | dependency injection instead
   421 | WARNING | \Drupal calls should be avoided in classes, use
       |         | dependency injection instead
  --------------------------------------------------------------------------
  
  
  FILE: testsuite.module
  ----------------------------------------------------------------------
  FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
  ----------------------------------------------------------------------
   26 | WARNING | Unused variable $reqVariables.
  ----------------------------------------------------------------------
  
  Time: 686ms; Memory: 14MB
  

• PHPStan has found some issues with your code). See attachment.

FILE: .../pareviewsh/pareview_temp/modules/list_packages/list_packages.module
--------------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 6 LINES
--------------------------------------------------------------------------
 11 | ERROR | [x] Line indented incorrectly; expected 0 spaces, found 1
 12 | ERROR | [x] Expected 2 space(s) before asterisk; 1 found
 13 | ERROR | [x] Expected 2 space(s) before asterisk; 1 found
 14 | ERROR | [x] Expected 2 space(s) before asterisk; 1 found
 15 | ERROR | [x] Expected 2 space(s) before asterisk; 1 found
 16 | ERROR | [x] Expected 2 space(s) before asterisk; 1 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 6 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...pareviewsh/pareview_temp/modules/list_packages/list_packages.install
--------------------------------------------------------------------------
FOUND 2 ERRORS AND 20 WARNINGS AFFECTING 22 LINES
--------------------------------------------------------------------------
   1 | ERROR   | [x] Missing file doc comment
  39 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 2
  45 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 0
 109 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 0
 149 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 2
 155 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 0
 219 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 0
 259 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 2
 265 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 0
 329 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 0
 374 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 1
 380 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 2
 386 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 0
 450 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 0
 490 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 1
 496 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 2
 502 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 0
 566 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 0
 650 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: FALSE
 655 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: FALSE
 689 | ERROR   | [x] Short array syntax must be used to define arrays
 693 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: 0
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 22 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...areviewsh/pareview_temp/modules/list_packages/list_packages.info.yml
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
--------------------------------------------------------------------------
 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
--------------------------------------------------------------------------


FILE: ...pareviewsh/pareview_temp/modules/phpunit_tests/phpunit_tests.install
--------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------
 169 | ERROR | [x] Expected 1 space(s) before asterisk; 0 found
 170 | ERROR | [x] Expected 1 space(s) before asterisk; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...areviewsh/pareview_temp/modules/phpunit_tests/phpunit_tests.info.yml
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
--------------------------------------------------------------------------
 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
--------------------------------------------------------------------------


FILE: ...ts/pareviewsh/pareview_temp/modules/phpcs_tests/phpcs_tests.info.yml
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
--------------------------------------------------------------------------
 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
--------------------------------------------------------------------------


FILE: ...cts/pareviewsh/pareview_temp/modules/phpcs_tests/phpcs_tests.install
--------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------
 78 | ERROR | [x] Expected 1 space(s) before asterisk; 2 found
 79 | ERROR | [x] Expected 1 space(s) before asterisk; 2 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...areviewsh/pareview_temp/modules/phpstan_tests/phpstan_tests.info.yml
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
--------------------------------------------------------------------------
 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
--------------------------------------------------------------------------


FILE: ...pareviewsh/pareview_temp/modules/phpstan_tests/phpstan_tests.install
--------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------
 77 | ERROR | [x] Expected 1 space(s) before asterisk; 2 found
 78 | ERROR | [x] Expected 1 space(s) before asterisk; 2 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: testsuite.install
----------------------------------------------------------------------
FOUND 3 ERRORS AND 1 WARNING AFFECTING 4 LINES
----------------------------------------------------------------------
  1 | ERROR   | [x] Missing file doc comment
  3 | WARNING | [x] Unused use statement
 68 | ERROR   | [x] Expected 1 space(s) before asterisk; 2 found
 69 | ERROR   | [x] Expected 1 space(s) before asterisk; 2 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: css/claro_theme.css
------------------------------------------------------------------------
FOUND 13 ERRORS AFFECTING 13 LINES
------------------------------------------------------------------------
  2 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  6 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  7 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
  8 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  9 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 10 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
 11 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 15 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 16 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
 17 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 18 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 19 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
 20 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
------------------------------------------------------------------------
PHPCBF CAN FIX THE 13 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------


FILE: css/filter_forms.css
--------------------------------------------------------------------------
FOUND 9 ERRORS AFFECTING 9 LINES
--------------------------------------------------------------------------
  1 | ERROR | [x] End of line character is invalid; expected "\n" but
    |       |     found "\r\n"
  2 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 14 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 18 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 22 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 26 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 30 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 34 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 38 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 9 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: css/seven_theme.css
------------------------------------------------------------------------
FOUND 26 ERRORS AFFECTING 26 LINES
------------------------------------------------------------------------
  7 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  8 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
  9 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 10 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 14 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 15 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 16 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 17 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 18 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 19 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 20 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
 21 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
 22 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
 23 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
 24 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 28 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 29 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 30 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 31 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 32 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 33 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
 34 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
 35 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
 36 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
 37 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
 38 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
------------------------------------------------------------------------
PHPCBF CAN FIX THE 26 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------


FILE: testsuite.info.yml
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
--------------------------------------------------------------------------
 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
--------------------------------------------------------------------------

Time: 809ms; Memory: 16MB

 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   pareview_temp/modules/list_packages/src/Controller/ListCoreLibrariesController.php
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------
  129    File list_packages.admin.inc could not be loaded from Drupal\Core\Extension\ModuleHandlerInterface::loadInclude because list_packages module is not found.
  263    Method Drupal\list_packages\Controller\ListCoreLibrariesController::createName() should return string but return statement is missing.
  395    File list_packages.admin.inc could not be loaded from Drupal\Core\Extension\ModuleHandlerInterface::loadInclude because list_packages module is not found.
  416    Variable $filter_where might not be defined.
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------

 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   pareview_temp/modules/list_packages/src/Controller/ListDashboardController.php
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------
  89     File list_packages.admin.inc could not be loaded from Drupal\Core\Extension\ModuleHandlerInterface::loadInclude because list_packages module is not found.
  534    Variable $route might not be defined.
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------

 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   pareview_temp/modules/list_packages/src/Controller/ListLibrariesController.php
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------
  155    File list_packages.admin.inc could not be loaded from Drupal\Core\Extension\ModuleHandlerInterface::loadInclude because list_packages module is not found.
  505    File list_packages.admin.inc could not be loaded from Drupal\Core\Extension\ModuleHandlerInterface::loadInclude because list_packages module is not found.
  518    Variable $filter_where in empty() always exists and is not falsy.
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------

 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   pareview_temp/modules/list_packages/src/Controller/ListModulesController.php
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------
  251    Method Drupal\list_packages\Controller\ListModulesController::createName() should return string but return statement is missing.
  273    File list_packages.admin.inc could not be loaded from Drupal\Core\Extension\ModuleHandlerInterface::loadInclude because list_packages module is not found.
  666    File list_packages.admin.inc could not be loaded from Drupal\Core\Extension\ModuleHandlerInterface::loadInclude because list_packages module is not found.
  679    Variable $filter_where in empty() always exists and is not falsy.
  709    File list_packages.admin.inc could not be loaded from Drupal\Core\Extension\ModuleHandlerInterface::loadInclude because list_packages module is not found.
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------

 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   pareview_temp/modules/list_packages/src/Controller/ListPackagesController.php
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------
  130    File list_packages.admin.inc could not be loaded from Drupal\Core\Extension\ModuleHandlerInterface::loadInclude because list_packages module is not found.
  392    File list_packages.admin.inc could not be loaded from Drupal\Core\Extension\ModuleHandlerInterface::loadInclude because list_packages module is not found.
  405    Variable $filter_where in empty() always exists and is not falsy.
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------

 ------ --------------------------------------------------------------------
  Line   pareview_temp/modules/list_packages/src/Form/LoadModsThemsForm.php
 ------ --------------------------------------------------------------------
  177    Class Drupal referenced with incorrect case: DRUPAL.
 ------ --------------------------------------------------------------------

 ------ -------------------------------------------------------------------
  Line   pareview_temp/modules/list_packages/src/Form/LoadPackagesForm.php
 ------ -------------------------------------------------------------------
  129    Variable $packages in isset() always exists and is not nullable.
 ------ -------------------------------------------------------------------

 ------ ----------------------------------------------------------------------------------------------------------------------------------
  Line   pareview_temp/modules/list_packages/src/ListPackagesResourceService.php
 ------ ----------------------------------------------------------------------------------------------------------------------------------
  244    Variable $version in isset() always exists and is not nullable.
  250    Variable $vulnerabilitiesInfo in isset() always exists and is not nullable.
  409    Variable $packageData might not be defined.
  420    \Drupal calls should be avoided in classes, use dependency injection instead
  421    \Drupal calls should be avoided in classes, use dependency injection instead
  588    Variable $newLibraries might not be defined.
  646    \Drupal calls should be avoided in classes, use dependency injection instead
  759    Method Drupal\list_packages\ListPackagesResourceService::createVulnerabilityArrayGithub() invoked with 2 parameters, 1 required.
 ------ ----------------------------------------------------------------------------------------------------------------------------------

 ------ --------------------------------------------------------------------------------
  Line   pareview_temp/modules/phpcs_tests/src/Form/PhpcsTestsRunTestsMultistepForm.php
 ------ --------------------------------------------------------------------------------
  359    Variable $types might not be defined.
  607    Variable $result might not be defined.
 ------ --------------------------------------------------------------------------------

 ------ ------------------------------------------------------------------------------------
  Line   pareview_temp/modules/phpstan_tests/src/Form/PhpstanTestsRunTestsMultistepForm.php
 ------ ------------------------------------------------------------------------------------
  381    Variable $types might not be defined.
 ------ ------------------------------------------------------------------------------------

 ------ --------------------------------------------------------
  Line   pareview_temp/src/Controller/CustomTestsController.php
 ------ --------------------------------------------------------
  298    Variable $filter_where might not be defined.
 ------ --------------------------------------------------------

 ------ ---------------------------------------------------------------------------------
  Line   pareview_temp/tests_module/custom_tests/src/CustomTSTests/CheckFor404Errors.php
 ------ ---------------------------------------------------------------------------------
  69     \Drupal calls should be avoided in classes, use dependency injection instead
 ------ ---------------------------------------------------------------------------------

 ------ ---------------------------------------------------------------------------------------
  Line   pareview_temp/tests_module/custom_tests/src/CustomTSTests/CheckForLibrariesModule.php
 ------ ---------------------------------------------------------------------------------------
  69     \Drupal calls should be avoided in classes, use dependency injection instead
 ------ ---------------------------------------------------------------------------------------

 [ERROR] Found 33 errors

README.md does not follow Drupal's template or best practices → .

Remove LICENSE.txt, it will be added by drupal.org packaging automatically.

The captchetat.module file does not implement hook_help() → .

 ------ ---------------------------------------------------------------------------------------------------
  Line   pareview_temp/src/Plugin/WebformElement/WebformCaptchetatElement.php
 ------ ---------------------------------------------------------------------------------------------------
  24     Class Drupal\captchetat\Plugin\WebformElement\WebformCaptchetatElement extends unknown class
         Drupal\webform\Plugin\WebformElementBase.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
  43     Parameter $webform_submission of method
         Drupal\captchetat\Plugin\WebformElement\WebformCaptchetatElement::prepare() has invalid type
         Drupal\webform\WebformSubmissionInterface.
  56     Drupal\captchetat\Plugin\WebformElement\WebformCaptchetatElement::form() calls parent::form() but
         Drupal\captchetat\Plugin\WebformElement\WebformCaptchetatElement does not extend any class.
 ------ ---------------------------------------------------------------------------------------------------

No automated test cases were found, did you consider writing Simpletests → or PHPUnit tests → ? This is not a requirement but encouraged for professional software development.

There are a lot of issues with the JS files.

jquery-captcha.js

MANY Unexpected var, use let or const instead
19:28 error '_getCaptchaStyleName' was used before it was defined
53:22 error '_getInstance' was used before it was defined (there are more lines where this happens)
92:1 Put options·||·{} on its own line
101:20 error '_onLoadScriptsSuccess' was used before it was defined
120:22 error 'NULL' is not defined
All 'var' declarations must be at the top of the function scope

And more. I recommend running ESLint on the JS files.

In addition, while this isn't related to the code itself, that old style of captcha can easily be read and defeated by an AI now. It's basically useless at verifying that someone is a human.

It would be nice if the README.md table of contents items were each linked to the corresponding section. Also, your README.md does not follow best practices (headings need to be uppercase). See https://www.drupal.org/node/2181737 → . There is also a line that it longer than 80 characters.

The module doesn't implement hook_help(). It'd be a good idea to do so.

basic-slider.html.twig is missing the comments at the top of the file. Look at any Drupal core twig file to see how to do it.

BasicSliderBlock.php

  /**
   * The storage handler class for files.
   *
   * @var \Drupal\file\FileStorage
   */
  private $fileStorage;

This should be typed as FileStorageInterface, in case someone wants to extend your class and use a custom file storage.

    $images = $this->configuration['images'];
    if (count($images)) {
      foreach ($images as $image) {
        if (!empty($image)) {
          if ($file = $this->fileStorage->load($image)) {
            $build['image'][] = [
              '#theme' => 'image',
              '#uri' => $file->getFileUri(),
            ];
          }
        }
      }
    }
    $build['#images'] = $build['image'];
    return $build;

$build['image'] could be undefinied here, if there are no images. Use $build['#images'] inside the for loop, and get rid of $build['image'], or you can check if it's empty before assigning it.

    if (count($images)) {
      foreach ($images as $image) {

You don't need the IF statement. The for loop will only run its code if there are images.

basic-slider.js

Consider rewriting it in vanilla JS without using jQuery. You barely used jQuery, so try going without. Drupal Core is working to remove jQuery, so it's a good idea to get rid of it if you can.

In addition, ESLint reports a lot of errors for this file. Please run ESLint against your project using Drupal's eslintrc.json. There are lots of spacing issues, you should use let or const instead of var, etc.

I installed a newer version of NodeJS, and that fixed it.

solideogloria → created an issue.

I recommend setting the 2.0.x branch (the latest branch) to be the default branch on GitLab. That way, merge requests will target the latest branch by default.

The MultistepForm::prepareWizard should have it's parameters documented in the function doc comment.

Your README.md does not follow best practices (headings need to be uppercase and sections are missing). See https://www.drupal.org/node/2181737 → . Also, lines should be wrapped at 80 characters.

The twig template hardcodes CDN links to 3rd party libraries:

    <!-- Highlight.js -->
    <script src="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.6.0/highlight.min.js"></script>
    <link rel="stylesheet" href="//cdnjs.cloudflare.com/ajax/libs/highlight.js/10.0.3/styles/default.min.css" />
    <link rel="stylesheet" href="//cdnjs.cloudflare.com/ajax/libs/highlight.js/10.0.3/styles/atom-one-light.min.css" />
    <script src="https://cdn.jsdelivr.net/gh/TRSasasusu/highlightjs-highlight-lines.js@1.1.5/highlightjs-highlight-lines.min.js"></script>

    <!-- Font Awesome -->
    <link href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.0.0/css/all.min.css" rel="stylesheet" />

    <!-- Google Fonts -->
    <link href="https://fonts.googleapis.com/css?family=Roboto:300,400,500,700&display=swap" rel="stylesheet" />

    <!-- MDB -->
    <link href="https://cdnjs.cloudflare.com/ajax/libs/mdb-ui-kit/4.4.0/mdb.min.css" rel="stylesheet" />
    <script type="text/javascript" src="https://cdnjs.cloudflare.com/ajax/libs/mdb-ui-kit/4.4.0/mdb.min.js"></script>

    <!-- jQuery -->
    <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.5.1/jquery.min.js"></script>

    <!-- DOMPurify -->
    <script src="https://cdn.jsdelivr.net/npm/dompurify@2.2.9/dist/purify.min.js"></script>

This is a bad way to do it. Use Drupal's library system instead.

Also, when the module is being uninstalled, you shouldn't need to update a config value, since the module's config is deleted when it is uninstalled.

/**
 * Implements hook_uninstall().
 *
 * Disable error reporting configuration for the module.
 */
function error_reporting_uninstall() {
  // Get the configuration service.
  $config = \Drupal::configFactory();

  // Set the 'enable_error_reporting' configuration value to FALSE.
  $config->getEditable('error_reporting.settings')
    ->set('enable_error_reporting', FALSE)
    ->save();
}

// Set a custom exception handler.
set_exception_handler('error_reporting_exception_handler');

/**
 * Custom exception handler function.
 */
function error_reporting_exception_handler(\Throwable $exception) {
  // Get the container.
  $container = \Drupal::getContainer();

  // Get the config service.
  $config = $container->get('config.factory')->get('error_reporting.settings')->get('enable_error_reporting');

  // If custom error reporting is disabled, let Drupal handle the exception.
  if (!$config) {
    // Create a Symfony Response object with a 500 status code and send it.
    $response = new Response('The website encountered an unexpected error. Please try again later.', Response::HTTP_INTERNAL_SERVER_ERROR);
    $response->send();
    return;
  }

The code says that if error reporting is disabled that it lets Drupal handle the exception, but that's not what it actually does. In order to let Drupal handle it, this code would need to take all of the error handler parameters and call Drupal's error handler, right?

_drupal_error_handler($error_level, $message, $filename, $line);

If you want more detailed feedback, you could try creating a survey of some kind.

I don't use GA. I use statistics on its own for internal standalone metrics with views integration.

Also, the MR is targeting the wrong branch. It should be against 2.x

Marking Critical, because the module's code looks absolutely terrible.

@solideogloria I don't know if unit fit's all the things I do with nginx config for now.

Me neither. I don't use it yet (still using PHP-FPM); I'm just looking at what's available.

That's all that I saw. Did anyone run PAReview.sh against the codebase?

This is a minor nuance, but there's a newer version of the .gitlab-ci.yml template available.

There are some coding standards issues. There should be a space before the return type declarations. There are more occurrences that need fixing, but here are some examples:

  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition):static {
    return new static($configuration, $plugin_id, $plugin_definition,
      $container->get('config.factory')
    );
  }

  /**
   * {@inheritdoc}
   */
  public function build():array {

Here's a post that compares the performance of PHP-FPM and Nginx Unit: Comparing PHP-FPM, NGINX Unit, and Laravel Octane

It looks like Nginx is moving towards Unit.

Here's the Nginx Unit recipe for Drupal: https://unit.nginx.org/howto/drupal/

I tested the MR using the new option and it works.

solideogloria → created an issue.

It'd be great if someone could review this, please.