- Issue created by @damien laguerre
- Status changed to Needs review
8 months ago 4:32pm 18 April 2024 - 🇮🇹Italy apaderno Brescia, 🇮🇹
Thank 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 smother review.
The important notes are the following.
- If you have not done it yet, you should run
phpcs --standard=Drupal,DrupalPractice
on the project, which alone fixes most of what reviewers would report. - For the time this application is open, only your commits are allowed.
- The purpose of this application is giving you a new drupal.org role that allows you to opt projects into security advisory coverage, either projects you already created, or projects you will create. The project status won't be changed by this application and no other user will be able to opt projects into security advisory policy.
- We only accept an application per user. If you change your mind about the project to use for this application, please update the issue summary to give the link to the correct project link and the issue title to contain that project name.
To the reviewers
Please read How to review security advisory coverage applications → , What to cover in an application review → , and Drupal.org security advisory coverage application workflow → .
Reviewers only describe what needs to be changed; they don't provide patches to fix what reported in a review. - If you have not done it yet, you should run
- Status changed to Needs work
8 months ago 11:13am 21 April 2024 - 🇮🇳India vishal.kadam Mumbai
1. Fix phpcs issues.
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml ipless/ FILE: ipless/ipless.routing.yml ---------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE ---------------------------------------------------------------------- 7 | ERROR | [x] Expected 1 newline at end of file; 0 found ---------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY ---------------------------------------------------------------------- FILE: ipless/ipless.libraries.yml ---------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE ---------------------------------------------------------------------- 12 | ERROR | [x] Expected 1 newline at end of file; 0 found ---------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY ---------------------------------------------------------------------- FILE: ipless/ipless.module ------------------------------------------------------------------------------ FOUND 3 ERRORS AND 1 WARNING AFFECTING 4 LINES ------------------------------------------------------------------------------ 8 | WARNING | [x] Unused use statement 9 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\ipless\Form\IpLessSettingForm. 71 | ERROR | [x] Data types in @var tags need to be fully namespaced 83 | ERROR | [x] Data types in @var tags need to be fully namespaced ------------------------------------------------------------------------------ PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------ FILE: ipless/src/Form/IpLessSettingForm.php ------------------------------------------------------------------------------ FOUND 10 ERRORS AND 6 WARNINGS AFFECTING 16 LINES ------------------------------------------------------------------------------ 10 | ERROR | [ ] Doc comment short description must be on a single line, further text should be a separate paragraph 21 | ERROR | [ ] Missing parameter comment 22 | ERROR | [ ] Missing parameter comment 36 | ERROR | [ ] Public method name "IpLessSettingForm::formAlter_system_performance_settings" is not in lowerCamel format 48 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead 54 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead 55 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead 61 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead 62 | ERROR | [x] Short array syntax must be used to define arrays 63 | ERROR | [x] Short array syntax must be used to define arrays 64 | ERROR | [x] Short array syntax must be used to define arrays 72 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead 73 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead 74 | ERROR | [x] Short array syntax must be used to define arrays 75 | ERROR | [x] Short array syntax must be used to define arrays 76 | ERROR | [x] Short array syntax must be used to define arrays ------------------------------------------------------------------------------ PHPCBF CAN FIX THE 6 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------ FILE: ipless/src/Event/IplessCompilationEvent.php ------------------------------------------------------------------------------ FOUND 3 ERRORS AND 1 WARNING AFFECTING 4 LINES ------------------------------------------------------------------------------ 6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\ipless\Asset\AssetRenderer. 9 | ERROR | [x] Doc comment short description must end with a full stop 11 | WARNING | [ ] @author tags are not usually used in Drupal, because over time multiple contributors will touch the code anyway 35 | ERROR | [ ] Description for the @return value is missing ------------------------------------------------------------------------------ PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------ FILE: ipless/src/Event/IplessEvents.php ------------------------------------------------------------------------------ FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES ------------------------------------------------------------------------------ 8 | ERROR | [x] Doc comment short description must end with a full stop 10 | WARNING | [ ] @author tags are not usually used in Drupal, because over time multiple contributors will touch the code anyway ------------------------------------------------------------------------------ PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------ FILE: ipless/src/IplessInterface.php ------------------------------------------------------------------------------ FOUND 9 ERRORS AND 1 WARNING AFFECTING 9 LINES ------------------------------------------------------------------------------ 9 | WARNING | [ ] The class short comment should describe what the class does and not simply repeat the class name 16 | ERROR | [x] Parameter comment must end with a full stop 28 | ERROR | [ ] Description for the @return value is missing 35 | ERROR | [ ] Missing parameter comment 35 | ERROR | [x] Data types in @param tags need to be fully namespaced 45 | ERROR | [ ] Description for the @return value is missing 57 | ERROR | [ ] Description for the @return value is missing 64 | ERROR | [ ] Description for the @return value is missing 71 | ERROR | [ ] Description for the @return value is missing 78 | ERROR | [ ] Description for the @return value is missing ------------------------------------------------------------------------------ PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------ FILE: ipless/src/Commands/IplessCommands.php ------------------------------------------------------------------------------ FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------------------------------------- 6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\ipless\Ipless. ------------------------------------------------------------------------------ PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------ FILE: ipless/src/EventSubscriber/HtmlResponseIplessSubscriber.php ------------------------------------------------------------------------------ FOUND 8 ERRORS AND 1 WARNING AFFECTING 8 LINES ------------------------------------------------------------------------------ 1 | ERROR | [x] The PHP open tag must be followed by exactly one blank line 11 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Symfony\Component\EventDispatcher\EventSubscriberInterface. 11 | ERROR | [x] There must be one blank line after the last USE statement; 2 found; 15 | ERROR | [x] Doc comment short description must end with a full stop 18 | WARNING | [ ] The class short comment should describe what the class does and not simply repeat the class name 21 | ERROR | [ ] Missing short description in doc comment 26 | ERROR | [ ] Missing short description in doc comment 34 | ERROR | [ ] Missing parameter comment 35 | ERROR | [ ] Missing parameter comment ------------------------------------------------------------------------------ PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------ FILE: ipless/src/Ipless.php ------------------------------------------------------------------------------ FOUND 18 ERRORS AFFECTING 18 LINES ------------------------------------------------------------------------------ 14 | ERROR | [x] There must be one blank line after the last USE statement; 2 found; 24 | ERROR | [ ] Missing short description in doc comment 29 | ERROR | [ ] Missing short description in doc comment 34 | ERROR | [ ] Missing short description in doc comment 39 | ERROR | [ ] Missing short description in doc comment 44 | ERROR | [ ] Missing short description in doc comment 49 | ERROR | [ ] Missing short description in doc comment 54 | ERROR | [ ] Missing short description in doc comment 59 | ERROR | [ ] Missing short description in doc comment 67 | ERROR | [ ] Missing parameter comment 68 | ERROR | [ ] Missing parameter comment 69 | ERROR | [ ] Missing parameter comment 70 | ERROR | [ ] Missing parameter comment 71 | ERROR | [ ] Missing parameter comment 72 | ERROR | [ ] Missing parameter comment 73 | ERROR | [ ] Missing parameter comment 95 | ERROR | [x] Missing function doc comment 116 | ERROR | [ ] Description for the @return value is missing ------------------------------------------------------------------------------ PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------ FILE: ipless/src/Controller/IplessController.php ------------------------------------------------------------------------------ FOUND 7 ERRORS AND 1 WARNING AFFECTING 8 LINES ------------------------------------------------------------------------------ 13 | ERROR | [x] Doc comment short description must end with a full stop 16 | WARNING | [ ] The class short comment should describe what the class does and not simply repeat the class name 19 | ERROR | [ ] Missing short description in doc comment 27 | ERROR | [ ] Missing parameter comment 53 | ERROR | [ ] Missing parameter comment 55 | ERROR | [ ] Description for the @return value is missing 69 | ERROR | [x] Expected newline after closing brace 76 | ERROR | [x] Expected 1 newline at end of file; 0 found ------------------------------------------------------------------------------ PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------ FILE: ipless/src/Asset/AssetRenderer.php ------------------------------------------------------------------------------ FOUND 5 ERRORS AND 1 WARNING AFFECTING 6 LINES ------------------------------------------------------------------------------ 6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Asset\LibraryDiscoveryInterface. 17 | ERROR | [x] Non-namespaced classes/interfaces/traits should not be referenced with use statements 18 | ERROR | [x] Non-namespaced classes/interfaces/traits should not be referenced with use statements 22 | WARNING | [ ] The class short comment should describe what the class does and not simply repeat the class name 43 | ERROR | [x] Doc comment short description must end with a full stop 144 | ERROR | [x] Expected newline after closing brace ------------------------------------------------------------------------------ PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------ FILE: ipless/src/Asset/AssetResolver.php ------------------------------------------------------------------------------ FOUND 6 ERRORS AND 1 WARNING AFFECTING 7 LINES ------------------------------------------------------------------------------ 7 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Component\Utility\Crypt. 22 | ERROR | [ ] Missing short description in doc comment 27 | ERROR | [ ] Missing short description in doc comment 65 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses 75 | ERROR | [x] 6 spaces found before inline comment; expected "// 'group' => LESS_AGGREGATE_DEFAULT," but found "// 'group' => | | LESS_AGGREGATE_DEFAULT," 83 | ERROR | [x] list(...) is forbidden, use [...] instead. 89 | WARNING | [x] '@todo: add group sort (as $group => $data)' should match the format '@todo Fix problem X here.' ------------------------------------------------------------------------------ PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY ------------------------------------------------------------------------------ FILE: ipless/src/Asset/AssetRendererInterface.php ------------------------------------------------------------------------------ FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES ------------------------------------------------------------------------------ 8 | WARNING | @author tags are not usually used in Drupal, because over time multiple contributors will touch the code anyway 9 | WARNING | The class short comment should describe what the class does and not simply repeat the class name ------------------------------------------------------------------------------
2. FILE: ipless.libraries.yml
version: VERSION
VERSION is only used by Drupal core modules. Contributed modules should use a literal string that does not change with the Drupal core version a site is using.
3. FILE: ipless.module
/** * @file * Adds the Less CSS compilation to Drupal. */
The 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.
4. FILE: src/Ipless.php
/** * Ipless constructor. * * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory * @param \Drupal\ipless\Asset\AssetRendererInterface $assetRenderer * @param \Drupal\Core\Asset\LibraryDiscoveryInterface $libraryDiscovery * @param \Drupal\Core\Extension\ModuleHandlerInterface $moduleHandler * @param \Drupal\Core\Extension\ThemeHandlerInterface $themeHandler * @param \Drupal\Core\File\FileSystemInterface $fileSystem * @param \Drupal\Core\State\StateInterface $state */ public function __construct(ConfigFactoryInterface $configFactory, AssetRendererInterface $assetRenderer, LibraryDiscoveryInterface $libraryDiscovery, ModuleHandlerInterface $moduleHandler, ThemeHandlerInterface $themeHandler, FileSystemInterface $fileSystem, StateInterface $state) {
FILE: src/Asset/AssetRenderer.php
/** * AssetRenderer constructor. * * @param \Drupal\Core\Extension\ThemeHandlerInterface $theme_handler * Theme Handler service. * @param \Drupal\Core\Asset\LibraryDiscoveryInterface $library_discovery * Library discover service. * @param \Drupal\Core\Theme\ThemeManagerInterface $theme_manager * Theme manager service. * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory * Config factory service. * @param \Drupal\ipless\Asset\AssetResolverInterface $asset_resolver * Asset resolver service. * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $event_dispatcher * Event dispatcher. * @param \Drupal\Core\Session\AccountProxyInterface $currentUser * The current user. * @param \Drupal\Core\File\FileSystemInterface $fileSystem * The file system service. */ public function __construct(ThemeHandlerInterface $theme_handler, LibraryDiscoveryInterface $library_discovery, ThemeManagerInterface $theme_manager, ConfigFactoryInterface $config_factory, AssetResolverInterface $asset_resolver, EventDispatcherInterface $event_dispatcher, AccountProxyInterface $currentUser, FileSystemInterface $fileSystem) {
FILE: src/Commands/IplessCommands.php
/** * IplessCommands constructor. * * @param \Drupal\ipless\Ipless $ipless * Ipless service. */ public function __construct(Ipless $ipless) {
FILE: src/Controller/IplessController.php
/** * IplessController constructor. * * @param \Drupal\ipless\IplessInterface $ipless */ public function __construct(IplessInterface $ipless) {
FILE: src/Event/IplessCompilationEvent.php
/** * The constructor. * * @param \Drupal\ipless\Asset\AssetRenderer $asset_renderer * Less AssetRenderer. */ public function __construct(AssetRenderer $asset_renderer) {
FILE: src/EventSubscriber/HtmlResponseIplessSubscriber.php
/** * HtmlResponseIplessSubscriber constructor. * * @param \Drupal\ipless\IplessInterface $ipless * @param \Drupal\ipless\Asset\AssetResolverInterface $assetResolver */ public function __construct(IplessInterface $ipless, AssetResolverInterface $assetResolver) {
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.
- Status changed to Needs review
5 months ago 4:39pm 19 July 2024 - 🇫🇷France damien laguerre
Thank you for your time and sorry for the delay.
I've made all the changes. Everything is visible on the repository, I haven't made any release yet.
- Status changed to Needs work
5 months ago 5:45pm 19 July 2024 - 🇮🇳India vishal.kadam Mumbai
1. FILE: src/Form/IpLessSettingForm.php
'#title' => t('Less compilation enabled'),
'#title' => t('Less developper mode'), '#description' => t('Compile Less file all the time.'),
'#title' => t('Enable SourceMap'),
'#title' => t('Enable watch mode'), '#description' => t('Refreshed CSS sheets without needing to reload the page.'),
t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
2. FILE: src/Event/IplessCompilationEvent.php
/** * IplessCompilationEvent used to indicate new compilation. * * @author Damien LAGUERRE */ class IplessCompilationEvent extends Event {
FILE: src/Event/IplessEvents.php
/** * Class event IplessEvent, list all ipless events. * * @author Damien LAGUERRE */ final class IplessEvents extends Event {
@author tags are not usually used in Drupal, because over time multiple contributors will touch the code anyway
- Status changed to Needs review
5 months ago 8:11am 20 July 2024 - 🇮🇳India vishal.kadam Mumbai
Rest seems fine to me.
Let’s wait for other reviewers to take a look and if everything goes fine, you will get the role.
- 🇮🇳India vishal.kadam Mumbai
I am changing priority as per Issue priorities → .
- Assigned to apaderno
- Status changed to RTBC
4 months ago 2:37pm 28 August 2024 - 🇮🇹Italy apaderno Brescia, 🇮🇹
/** * The config factory service. * * @var \Drupal\Core\Config\ConfigFactoryInterface */ protected $configFactory; /** * The config object. * * @var \Drupal\Core\Config\ImmutableConfig */ protected $config;
Class instances do not need both those properties. Either the first is used, and a configuration object is created on methods when it is necessary, or the configuration object is created in the constructor and the configuration factory is never stored in a class property.
- Status changed to Fixed
4 months ago 2:38pm 28 August 2024 - 🇮🇹Italy apaderno Brescia, 🇮🇹
Thank you for your contribution!
I updated your account so you can now opt into security advisory coverage for any project you created and every project you will create.These are some recommended readings to help you with maintainership:
- Dries → ' post on Responsible maintainers
- Maintainership →
- Git version control system →
- Issue procedures and etiquette →
- Maintaining and responding to issues for a project →
- Release naming conventions → .
You can find more contributors chatting on Slack → or IRC → in #drupal-contribute. So, come hang out and stay involved → !
Thank you for your patience with the review process. 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 the dedicated reviewers as well.
- 🇫🇷France damien laguerre
Thank you for your help and the time you spent on my module.
I really appreciate it!Thanks again.
Automatically closed - issue fixed for 2 weeks with no activity.