- Issue created by @samit.310@gmail.com
- Issue was unassigned.
- Status changed to Needs review
almost 2 years ago 4:35am 27 March 2023 - š®š³India samit.310@gmail.com
Above errors/warnings has been fixed.
- Assigned to akshaydalvi212
- Issue was unassigned.
- Status changed to Needs work
almost 2 years ago 7:44am 27 March 2023 - š®š¹Italy apaderno Brescia, š®š¹
/** - * + * The Derivative class for RelevantContentBlock. */ class RelevantContentBlock extends DeriverBase implements ContainerDeriverInterface {
Derivative is not spelled correctly.
That description just repeats what the code already says clear and it does not describe the class purpose./** - * + * Constructs a new RelevantContentBlock instance. */
The class namespace is missing.
Parameters are not documented. - Assigned to akshaydalvi212
- š®š³India akshaydalvi212
As per #4 suggestion i will provide the patch for solving remaining issues as suggested.
- Issue was unassigned.
- Status changed to Needs review
almost 2 years ago 8:22am 27 March 2023 - š®š³India akshaydalvi212
fixed the suggested issues for coding standards.
- Status changed to Needs work
almost 2 years ago 9:00am 27 March 2023 - š®š¹Italy apaderno Brescia, š®š¹
/** - * + * The RelevantContentBlock class to get relevant content from storage. */
It is not necessary to repeat the class name. Probably it is not even necessary to say class.
to get relevant content from storage should probably be expanded a little, to make clear which content that is. from storage is probably not necessary./** + * Constructs a new RelevantContentBlock instance. * + * @param \Drupal\Core\Entity\EntityStorageInterface $preset_storage + * The preset stoage instance. */ public function __construct(EntityStorageInterface $preset_storage) {
The namespace is still missing.
- Status changed to Needs review
almost 2 years ago 1:50pm 27 March 2023 - š®š³India akshaydalvi212
updated the class comment as per suggestion and also added the namespace.
kindly review. - Status changed to RTBC
almost 2 years ago 5:30pm 27 March 2023 - š®š³India TanujJain-TJ
Tested patch #8 on drupal: 10.1.x, the patch applied successfully and removed all the errors reported by phpcs, and addresses all the points mentioned in #4 and #7. RTBC
- Status changed to Needs work
almost 2 years ago 11:44am 28 March 2023 - š®š¹Italy apaderno Brescia, š®š¹
The patch could apply, but it is wrong.
namespace Drupal\relevant_content\Plugin\Derivative; +namespace Drupal\Core\Entity\EntityStorageInterface; +
There must be a single line defining the namespace.
Then, contributed modules do not use namespaces starting withDrupal\Core
./** - * + * To get relevant content list as per the categorization of current page. */
Remove To.
/** + * Constructs a new RelevantContentBlock instance. * + * @param \Drupal\Core\Entity\EntityStorageInterface $preset_storage + * The preset stoage instance. */
The class namespace is missing.
There is a typo in the parameter description. Furthermore, it is not clear what a preset storage would be; The entity storage service. is probably better./** * Creates a Relevant Content Service class. + * + * @param \Symfony\Component\DependencyInjection\ContainerInterface $container + * The container service. + * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $event_dispatcher + * The eventDispatcher service. + * @param \Drupal\Core\Routing\RouteMatchInterface $route_match + * The route match service. + * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager + * The entity type manager service.
Since that comment is changed, also the first line needs to be changed.
- Assigned to akshaydalvi212
- Issue was unassigned.
- Status changed to Needs review
almost 2 years ago 6:44am 29 March 2023 - š®š³India akshaydalvi212
Providing #12 patch with all the changes mentioned by @apaderno in #10.
@apaderno thanks for all the suggestions.
kindly review the patch. Hi, Reviewed the patch #12 , applied cleanly fixes all the phpcs error/warnings and addresses the comments in #10.
- Status changed to Needs work
6 months ago 2:20am 16 July 2024 Hi @akshaydalvi212,
Applied your patch, it was applied successfully however, it threw few errors. Please see below:
relevant_content git:(1.x) curl https://www.drupal.org/files/issues/2023-03-29/3350530-12.patch | patch -p1 % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 4806 100 4806 0 0 15268 0 --:--:-- --:--:-- --:--:-- 15913 patching file relevant_content.services.yml patching file src/Plugin/Derivative/RelevantContentBlock.php patching file src/RelevantContentService.php ā relevant_content git:(1.x) ā cd .. ā contrib git:(main) ā phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig relevant_content FILE: ...ules/contrib/relevant_content/src/Form/RelevantContentPresetDeleteForm.php -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 7 | ERROR | [x] Use statements should be sorted alphabetically. The first | | wrong one is Drupal\Core\Form\FormStateInterface. -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: ...gissue/web/modules/contrib/relevant_content/src/RelevantContentService.php -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 10 | ERROR | [x] Use statements should be sorted alphabetically. The first | | wrong one is Drupal\Core\Entity\EntityTypeManagerInterface. -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- Time: 286ms; Memory: 10MB
Kindly check
Thanks,
Jake