Closed (fixed)
Project:
Drupal.org security advisory coverage applications
Component:
module
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
10 Jul 2025 at 00:52 UTC
Updated:
22 Oct 2025 at 12:54 UTC
Jump to comment: Most recent
Comments
Comment #2
vishal.kadamComment #3
avpadernoThank you for applying!
Please read Review process for security advisory coverage: What to expect for more details and Security advisory coverage application checklist to understand what reviewers look for. Tips for ensuring a smooth review gives some hints for a smoother review.
The important notes are the following.
To the reviewers
Please read How to review security advisory coverage applications, Application workflow, What to cover in an application review, and Tools to use for reviews.
The important notes are the following.
For new reviewers, I would also suggest to first read In which way the issue queue for coverage applications is different from other project queues.
Comment #4
vishal.kadam1. FILE: src/Commands/MetadataSanitizerCommands.php
The documentation comment for constructors is not mandatory anymore, If it is given, the description must be “Constructs a new [class name] object”, where [class name] includes the class namespace.
2. FILE: README.md
The README file is missing the required section - Configuration.
3. FILE: metadata_sanitizer.module
The usual description for a .module file is “Hook implementations for the [module name] module”, where [module name] is the module name given in the .info.yml file.
Comment #5
zarabatana commentedThe requested changes have been committed to the 1.0.x branch.
Please let me know if further updates are needed or if this is ready for the next step in the review process.
Comment #6
bbu23Hi! Thanks for applying.
Here's my feedback:
1. Convert Drush commands from PHP Annotations into PHP Attributes: https://www.drush.org/12.x/commands/.
2. There is no dev release.
3. The recommended way of installing contrib modules is using composer. I would modify the README.md file to recommend to "Install as you would normally install a contributed Drupal module." and for more information to link here. I wouldn't recommend unzipping as a first choice.
4. The
composer.jsoncould use some corrections/improvements:- Add license:
"license": "GPL-2.0-or-later"- The PHP requirement currently set to
">=7.4"is redundant. Drupal 10 requires PHP8+, therefore PHP 7 is impossible.- You could add a
"suggest"section for theexiftoolexternal tool.5. The functional test doesn't really bring any value, it could be improved.
Comment #7
zarabatana commentedAll requested changes have been completed:
Please let me know if anything else is required, or if this is ready for the next review step.
Comment #8
avpadernosrc/Form/MetadataSanitizerSettingsForm.php
ConfigFormBase::__construct()needs to be called. Since its parameters changed in Drupal 10.2, the project cannot be compatible with all the Drupal 10 releases and Drupal 11; it needs to require at least Drupal 10.2.With Drupal 10 and Drupal 11, there is no longer need to use
#default_valuefor each form element, when the parent class isConfigFormBase: It is sufficient to use#config_target, as in the following code.Using that code, it is no longer needed to save the configuration values in the form submission handler: The parent class will take care of that.
metadata_sanitizer.module
For a new module that aims to be compatible with Drupal 10 and Drupal 11, I would rather implement hooks as class methods as described in Support for object oriented hook implementations using autowired services.
Comment #9
zarabatana commentedThanks for the detailed review.
I’ve updated the settings form to use #config_target instead of #default_value, which also removes the need for manually saving values in the submit handler.
I also removed the redundant constructor override in the form.
In addition, the hook implementations were moved into a service-based class following the object-oriented hooks pattern with autowiring.
These changes should address the issues raised and bring the module in line with current Drupal 10/11 practices.
Comment #10
avpadernoThank you for your contribution and for your patience with the review process!
I am going to update your account so you can opt into security advisory coverage any project you create, including the projects you already created.
These are some recommended readings to help you with maintainership:
You can find more contributors chatting on Slack or IRC in #drupal-contribute. So, come hang out and stay involved!
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.
I thank also all the reviewers for helping with these applications.
Comment #11
avpaderno