Needs review
Project:
Drupal.org security advisory coverage applications
Component:
module
Priority:
Major
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
11 Feb 2026 at 07:12 UTC
Updated:
7 Apr 2026 at 06:16 UTC
Jump to comment: Most recent
Comments
Comment #2
vishal.kadamComment #3
rushikesh raval commentedThank 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.
phpcs --standard=Drupal,DrupalPracticeon the project, which alone fixes most of what reviewers would report.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: composer.json
As a side note, it is not necessary to add the Drupal core requirements in the /composer.json/ file: The Drupal.org Composer Façade will add them.
2. FILE: hivo_connector.info.yml
package: CustomThis line is used by custom modules created for specific sites. It is not a package name used for projects hosted on drupal.org.
core_version_requirement: ^9 || ^10 || ^11A new project should not declare itself compatible with a Drupal release that is no longer supported. No site should be using Drupal 8 nor Drupal 9, and people should not be encouraged to use those Drupal releases.
Comment #5
dannie_hivo commentedHello Vishal,
Sorry for the late reply. We just got back from the holiday.
We’ve fixed all the issues. Could you please take a look?
Comment #6
vishal.kadam1. FILE: src/Commands/HivoCleanupCommand.php
$this->logger()->error(" ✗ {$error_msg}");$this->logger()->error(" Errors/Warnings: " . count($errors));The
$messageparameter passed to theLoggerInterfacemethods must be a literal string that uses placeholders. It is not a translatable string returned fromt()/$this->t(), a string concatenation, a value returned from a function/method, nor a variable containing an exception object.2. FILE: src/Controller/HivoMediaApiController.php
FILE: src/Controller/HivoSyncApiController.php
FILE: src/Controller/HivoUploadApiController.php
New modules, which are compatible with Drupal 10 and higher versions are expected to include type declarations in property definitions, and use constructor property promotion.
Comment #7
dannie_hivo commentedHello
We fixed all issues, please check it.
Thank you
Comment #8
vishal.kadamRest seems fine to me.
Please wait for other reviewers and Project Moderator to take a look and if everything goes fine, you will get the role.
Comment #9
bbu23Comment #10
bbu23Hello,
Below you have my feedback:
- There is no dev release for your project. While not mandatory, it's a highly encouraged practice in the Drupal community. You should consider creating one.
- The composer.json file needs to be reviewed as it contains sections that don't belong there. See Add a composer.json file for more info.
- The Drush command definition and the PHP plugin files in
src/Pluginfolder should use PHP Attributes instead of Annotations for plugin definitions as described in Attribute-based plugins.- MetadataWidget.php: Line 96: It is incorrect to ignore the PHPCS warning about unused variable. The correct approach is to delete the actual unused variable with the funny name
$_.- There are still unsolved PHPCS warnings/errors
- The Helpers directory relies on static classes, which bypasses Drupal's dependency injection container. Classes like
HivoApiClient,S3ChunkedUploadHelperetc. should be registered as services inhivo_connector.services.ymland injected where needed — this makes them testable, swappable, and consistent with Drupal's architecture. See https://www.drupal.org/docs/drupal-apis/services-and-dependency-injectio......and that's all I have time for now.
Comment #11
dannie_hivo commentedHello
Thank you for your feedbacks.
I updated the repo.
Please check
Comment #12
dannie_hivo commentedHello
Anyone can check this please?
Comment #13
dannie_hivo commentedHello
Anyone can check this please?
Comment #14
vishal.kadamI am changing priority as per Issue priorities.