- Issue created by @droath
- 🇮🇳India vishal.kadam Mumbai
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 smoother review.
The important notes are the following.
- If you have not done it yet, you should run
phpcs --standard=Drupal,DrupalPracticeon 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, or it is necessary to use a different project for the application, please update the issue summary with the link to the correct project and the issue title with the project name and the branch to review.
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.
- It is preferable to wait for a Code Review Administrator before commenting on newly created applications. Code Review Administrators will do some preliminary checks that are necessary before any change on the project files is suggested.
- Reviewers should show the output of a CLI tool → only once per application.
- It may be best to have the applicant fix things before further review.
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 → .
- If you have not done it yet, you should run
- 🇩🇪Germany Anybody Porta Westfalica
Hi all, I reviewed the module code some days ago already as we planned using the module in several projects and just did a review again using https://www.drupal.org/node/1587704 →
I think the module is a great and couldn't find any major issues, just a little potential bug: 🐛 Wrong theme hook suggestion? Active
And some code style issues: 📌 Write (basic) automated tests RTBC (I think it would be great to have these tests and further tests added once the application here as been resolved)
The module uses several Drupal API functionalities like
- Theme System
- Plugin System & Services
- Form API
- Field API
- Submodules
- ...
and is a super useful contrib extension, so clear +1 on RTBC once the code style issues have been resolved. ✅
Thank you @droath! GREAT WORK!
- Status changed to Needs work
8 months ago 2:43pm 15 August 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 entity_extra_field/ FILE: entity_extra_field/entity_extra_field.module -------------------------------------------------------------------------------- FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES -------------------------------------------------------------------------------- 11 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Entity\EntityStorageInterface. 128 | WARNING | [x] A comma should follow the last multiline array item. Found: ) -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: entity_extra_field/modules/entity_extra_field_ui/src/Plugin/Derivative/MenuLinksTask.php -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 20 | ERROR | Missing short description in doc comment -------------------------------------------------------------------------------- FILE: entity_extra_field/modules/entity_extra_field_ui/src/Plugin/Derivative/MenuLinksAction.php -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 20 | ERROR | Missing short description in doc comment -------------------------------------------------------------------------------- FILE: entity_extra_field/modules/entity_extra_field_ui/src/Routing/RouteSubscriber.php -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 17 | ERROR | Missing short description in doc comment -------------------------------------------------------------------------------- FILE: entity_extra_field/README.md ---------------------------------------------------------------------- FOUND 0 ERRORS AND 11 WARNINGS AFFECTING 11 LINES ---------------------------------------------------------------------- 4 | WARNING | Line exceeds 80 characters; contains 222 characters 6 | WARNING | Line exceeds 80 characters; contains 264 characters 14 | WARNING | Line exceeds 80 characters; contains 180 characters 16 | WARNING | Line exceeds 80 characters; contains 168 characters 21 | WARNING | Line exceeds 80 characters; contains 249 characters 25 | WARNING | Line exceeds 80 characters; contains 116 characters 27 | WARNING | Line exceeds 80 characters; contains 101 characters 29 | WARNING | Line exceeds 80 characters; contains 109 characters 35 | WARNING | Line exceeds 80 characters; contains 156 characters 39 | WARNING | Line exceeds 80 characters; contains 234 characters 41 | WARNING | Line exceeds 80 characters; contains 138 characters ---------------------------------------------------------------------- FILE: entity_extra_field/src/ExtraFieldTypePluginInterface.php -------------------------------------------------------------------------------- FOUND 5 ERRORS AFFECTING 5 LINES -------------------------------------------------------------------------------- 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Component\Plugin\ConfigurableInterface. 44 | ERROR | [x] Function comment short description must start with exactly one space 50 | ERROR | [x] Return comment indentation must be 3 spaces, found 4 spaces 52 | ERROR | [x] Expected 1 blank line after function; 0 found 53 | ERROR | [x] The closing brace for the interface must have an empty line before it -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: entity_extra_field/src/Plugin/ExtraFieldType/ExtraFieldBlockPlugin.php -------------------------------------------------------------------------------- FOUND 3 ERRORS AND 1 WARNING AFFECTING 4 LINES -------------------------------------------------------------------------------- 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Plugin\Context\Context. 43 | ERROR | [ ] Missing short description in doc comment 48 | ERROR | [ ] Missing short description in doc comment 271 | WARNING | [x] A comma should follow the last multiline array item. Found: ) -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: entity_extra_field/src/Plugin/ExtraFieldType/ExtraFieldEntityLinkPlugin.php -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 9 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Access\AccessResult. -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: entity_extra_field/src/Plugin/ExtraFieldType/ExtraFieldTokenPlugin.php -------------------------------------------------------------------------------- FOUND 2 ERRORS AFFECTING 2 LINES -------------------------------------------------------------------------------- 67 | ERROR | [x] Array indentation error, expected 10 spaces but found 8 68 | ERROR | [x] Array indentation error, expected 10 spaces but found 8 -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: entity_extra_field/src/Plugin/ExtraFieldType/ExtraFieldComponentPlugin.php -------------------------------------------------------------------------------- FOUND 8 ERRORS AND 3 WARNINGS AFFECTING 11 LINES -------------------------------------------------------------------------------- 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Component\Utility\Unicode. 32 | ERROR | [ ] Missing short description in doc comment 167 | WARNING | [ ] Only string literals should be passed to t() where possible 180 | WARNING | [x] A comma should follow the last multiline array item. Found: NULL 185 | ERROR | [x] Expected newline after closing brace 218 | ERROR | [x] Expected 1 space after USE keyword; found 0 243 | WARNING | [x] A comma should follow the last multiline array item. Found: ] 281 | ERROR | [x] Return comment indentation must be 3 spaces, found 4 spaces 356 | ERROR | [x] Expected newline after closing brace 381 | ERROR | [x] Expected 1 blank line after function; 0 found 382 | ERROR | [x] The closing brace for the class must have an empty line before it -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 9 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: entity_extra_field/src/Plugin/ExtraFieldType/ExtraFieldViewsPlugin.php -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\views\ViewEntityInterface. -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: entity_extra_field/src/ExtraFieldTypePluginManager.php -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Cache\CacheBackendInterface. -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: entity_extra_field/src/EntityExtraFieldContextTrait.php -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Plugin\Context\ContextHandlerInterface. -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: entity_extra_field/src/Form/EntityExtraFieldFormDelete.php -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Cache\CacheBackendInterface. -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: entity_extra_field/src/Form/EntityExtraFieldForm.php -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 9 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Entity\EntityInterface. -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: entity_extra_field/src/Entity/EntityExtraField.php -------------------------------------------------------------------------------- FOUND 16 ERRORS AND 4 WARNINGS AFFECTING 20 LINES -------------------------------------------------------------------------------- 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Component\Utility\NestedArray. 64 | ERROR | [ ] Missing short description in doc comment 69 | ERROR | [ ] Missing short description in doc comment 74 | ERROR | [ ] Missing short description in doc comment 79 | ERROR | [ ] Missing short description in doc comment 84 | ERROR | [ ] Missing short description in doc comment 89 | ERROR | [ ] Missing short description in doc comment 94 | ERROR | [ ] Missing short description in doc comment 99 | ERROR | [ ] Missing short description in doc comment 104 | ERROR | [ ] Missing short description in doc comment 109 | ERROR | [ ] Missing short description in doc comment 114 | ERROR | [ ] Missing short description in doc comment 119 | ERROR | [ ] Missing short description in doc comment 124 | ERROR | [ ] Missing short description in doc comment 141 | WARNING | [x] A comma should follow the last multiline array item. Found: name 288 | WARNING | [x] A comma should follow the last multiline array item. Found: ) 300 | WARNING | [x] A comma should follow the last multiline array item. Found: ) 356 | ERROR | [x] Array indentation error, expected 10 spaces but found 8 357 | ERROR | [x] Array indentation error, expected 10 spaces but found 8 424 | WARNING | [x] A comma should follow the last multiline array item. Found: ) -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 7 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: entity_extra_field/src/Entity/EntityExtraFieldInterface.php -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 9 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Config\Entity\ConfigEntityInterface. -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: entity_extra_field/src/ExtraFieldTypePluginBase.php -------------------------------------------------------------------------------- FOUND 8 ERRORS AFFECTING 8 LINES -------------------------------------------------------------------------------- 6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Component\Utility\NestedArray. 27 | ERROR | [ ] Missing short description in doc comment 32 | ERROR | [ ] Missing short description in doc comment 37 | ERROR | [ ] Missing short description in doc comment 42 | ERROR | [ ] Missing short description in doc comment 47 | ERROR | [ ] Missing short description in doc comment 430 | ERROR | [x] Expected 1 blank line after function; 0 found 431 | ERROR | [x] The closing brace for the class must have an empty line before it -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: entity_extra_field/src/Annotation/ExtraFieldType.php -------------------------------------------------------------------------------- FOUND 2 ERRORS AFFECTING 2 LINES -------------------------------------------------------------------------------- 16 | ERROR | Missing short description in doc comment 21 | ERROR | Missing short description in doc comment -------------------------------------------------------------------------------- FILE: entity_extra_field/src/Controller/EntityExtraFieldListBuilder.php -------------------------------------------------------------------------------- FOUND 7 ERRORS AFFECTING 7 LINES -------------------------------------------------------------------------------- 9 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Entity\EntityInterface. 22 | ERROR | [ ] Missing short description in doc comment 27 | ERROR | [ ] Missing short description in doc comment 39 | ERROR | [ ] Doc comment for parameter $current_route_match does not match actual variable name <undefined> 111 | ERROR | [x] Expected newline after closing brace 172 | ERROR | [x] Expected 1 blank line after function; 0 found 173 | ERROR | [x] The closing brace for the class must have an empty line before it -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY --------------------------------------------------------------------------------2. FILE: README.md
The module uses a README.md file instead of a README.txt file. While the Drupal coding standards have not been yet updated about that, the Drupal.org community consider that positive.
Since there is a README.md file, that should follow the content and formatting described in README.md template → .3. FILE: src/ExtraFieldTypePluginBase.php
/** * Extra field type view constructor. * * @param array $configuration * The plugin configuration. * @param string $plugin_id * The plugin identifier. * @param array $plugin_definition * The plugin definition. * @param \Drupal\Core\Utility\Token $token * The token service. * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler * The module handler service. * @param \Drupal\Core\Routing\RouteMatchInterface $current_route_match * The current route match service. * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager * The entity type manager service. * @param \Drupal\Core\Entity\EntityFieldManagerInterface $entity_field_manager * The entity field manager service. */ public function __construct(FILE: src/ExtraFieldTypePluginManager.php
/** * {@inheritdoc} */ public function __construct(FILE: src/Form/EntityExtraFieldForm.php
/** * Define the extra field type manager. * * @param \Drupal\Core\Cache\CacheBackendInterface $cache_discovery_backend * The cache discovery backend service. * @param \Drupal\Core\Plugin\Context\ContextRepositoryInterface $context_repository * The context repository service. * @param \Drupal\Component\Plugin\PluginManagerInterface $extra_field_type_manager * The extra field type plugin manager. * @param \Drupal\Component\Plugin\PluginManagerInterface $condition_plugin_manager * The condition plugin manager. * @param \Drupal\Core\Cache\CacheTagsInvalidatorInterface $cache_tags_invalidator * The cache tags invalidator service. * @param \Drupal\Core\Entity\EntityDisplayRepositoryInterface $entity_display_repository * The entity display repository. */ public function __construct(FILE: src/Form/EntityExtraFieldFormDelete.php
/** * Entity extra field form delete constructor. */ public function __construct(CacheBackendInterface $cache_discovery) {FILE: src/Plugin/ExtraFieldType/ExtraFieldBlockPlugin.php
/** * Extra field block plugin constructor. * * @param array $configuration * The plugin configuration. * @param string $plugin_id * The plugin identifier. * @param array $plugin_definition * The plugin definition. * @param \Drupal\Core\Utility\Token $token * The token service. * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler * The module handler service. * @param \Drupal\Core\Session\AccountInterface $current_user * The current logged-in user object. * @param \Drupal\Core\Routing\RouteMatchInterface $current_route_match * The current route match service. * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager * The entity type manager service. * @param \Drupal\Core\Entity\EntityFieldManagerInterface $entity_field_manager * The entity field manager service. * @param \Drupal\Core\Block\BlockManagerInterface $block_manager * The block manager service. */ public function __construct(FILE: src/Plugin/ExtraFieldType/ExtraFieldComponentPlugin.php
/** * Define the class constructor. * * @param array $configuration * The plugin configuration. * @param string $plugin_id * The plugin identifier. * @param array $plugin_definition * The plugin definition. * @param \Drupal\Core\Utility\Token $token * The token service. * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler * The module handler service. * @param \Drupal\Core\Routing\RouteMatchInterface $current_route_match * The current route match service. * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager * The entity type manager service. * @param \Drupal\Core\Entity\EntityFieldManagerInterface $entity_field_manager * The entity field manager service. * @param \Drupal\sdc\ComponentPluginManager $component_plugin_manager * The component plugin manager service. */ public function __construct(FILE: src/Plugin/ExtraFieldType/ExtraFieldEntityLinkPlugin.php
/** * Extra field type view constructor. * * @param array $configuration * The plugin configuration. * @param string $plugin_id * The plugin identifier. * @param mixed $plugin_definition * The plugin definition. * @param \Drupal\Core\Utility\Token $token * The token service. * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler * The module handler service. * @param \Drupal\Core\Routing\RouteMatchInterface $current_route_match * The current route match service. * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager * The entity type manager service. * @param \Drupal\Core\Entity\EntityFieldManagerInterface $entity_field_manager * The entity field manager service. * @param \Drupal\Core\Access\AccessManagerInterface $access_manager * The access manager service. */ public function __construct(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.
- 🇮🇹Italy apaderno Brescia, 🇮🇹
The project page needs to mention the existing, similar projects, such as the Extra Field → module, and explain the differences between this module and the other ones.