- ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
The issue summary should always describe what the issue is trying to fix and, in the case, of coding standards issues, report which command has been used, which arguments have been used, and which report that command shown.
- ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
protected function getGroupMenus(GroupInterface $group) { - return \Drupal::service('groupmenu.menu')->loadUserGroupMenusByGroup('view', $group->id()); + return $this->service('groupmenu.menu')->loadUserGroupMenusByGroup('view', $group->id()); }
There is any
service()
method for that class or its parent classes.+ $tree = $this->menuTree()->load($menu_name, $parameters); + $tree = $this->menuTree()->transform($tree, $manipulators);
That method does not exist either.
- foreach ($this->addPageBundles($group, $create_mode) as $plugin_id => $bundle_name) { + foreach ($this->addPageBundles($group, $create_mode) as => $bundle_name) {
That is not correct PHP code.
/** - * Class GroupMenuSettingsForm. + * Class of GroupMenuSettingsForm. */
Adding a preposition is not sufficient to write a correct documentation comment. The comment is still just showing the class name, not its purpose.
// We can't use dependency injection for entity type manager, since this // will cause circular dependencies. - $entity_type_manager = \Drupal::service('entity_type.manager'); + $entity_type_manager = $this->service('entity_type.manager');
See the comment before the code. The maintainer is explaining why dependency injection has not been used.
- Assigned to kkalashnikov
- Issue was unassigned.
- ๐ฎ๐ณIndia kkalashnikov Ghaziabad, India
Fixed rest of the phpcs issues and include #12 comment as well.
- Status changed to Needs review
over 1 year ago 6:51am 23 March 2023 - Status changed to Needs work
over 1 year ago 12:22pm 23 March 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
+ /** + * Group menu service. + * + * @var \Drupal\groupmenu\GroupMenuService + */ + protected $groupMenuService; + + /** + * Menu link tree service. + * + * @var \Drupal\Core\Menu\MenuLinkTreeInterface + */ + protected $menuTree;
The descriptions are each missing an article.
+ /** + * Constructs a new group menu block. + *
The comment does not show the class name nor its namespace.
- // We can't use dependency injection for entity type manager, since this - // will cause circular dependencies. - $entity_type_manager = \Drupal::service('entity_type.manager'); - $plugin_id = 'group_menu:menu'; - $group_content_types = $entity_type_manager->getStorage('group_content_type') + $group_content_types = $this->entityTypeManager->getStorage('group_content_type')
We can't use dependency injection for entity type manager, since this will cause circular dependencies.
- Status changed to Needs review
over 1 year ago 8:22am 27 March 2023 - Status changed to Needs work
over 1 year ago 9:04am 27 March 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
+ /** + * Constructs a group menu block. + * + * @param \Drupal\groupmenu\GroupMenuService $groupMenuService
The class name and its namespace are still missing from the short description.
/** - * Constructs a new GroupMenuController. + * Constructs a GroupMenuController. *
The namespace is still missing.
- First commit to issue fork.
- Status changed to Needs review
over 1 year ago 9:34am 31 March 2023 - ๐ฎ๐ณIndia Mahima_Mathur23
Changes suggested by @aparderno has been incorporated in the MR changes done in #20 by @mukesh88.
- Status changed to Needs work
9 months ago 7:39pm 13 March 2024 - ๐ณ๐ฑNetherlands seanB Netherlands
Thanks for this. There is 1 thing that I noticed, the entity type manager is now injected when we explicitly didn't do that before to prevent a circular dependency.
- Assigned to nitin_lama
- Status changed to Needs review
9 months ago 10:15am 14 March 2024 - Issue was unassigned.
- ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
apaderno โ changed the visibility of the branch 3305893-phpcs-coding-standard to hidden.
- Status changed to Needs work
9 months ago 2:41pm 14 March 2024 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
- First commit to issue fork.
- ๐ฎ๐ณIndia pray_12
There is one error that needs to be addressed.
FILE: /groupmenu/src/GroupMenuConfigOverrides.php ------------------------------------------------------------------------------------------------------ FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE ------------------------------------------------------------------------------------------------------ 190 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead ------------------------------------------------------------------------------------------------------
- Status changed to Needs review
9 months ago 6:05am 18 March 2024 - ๐ฎ๐ณIndia nitin_lama India
@pray_12, as per comment #23. we are not sure if DI issue is fixed or not. For now i have reverted the DI for entity type manager.
- ๐ฎ๐ณIndia dev16.addweb
silvi.addweb โ made their first commit to this issueโs fork.
- Status changed to RTBC
5 months ago 6:38am 11 July 2024 Hi
Applied latest changes on MR !6, confirmed no errors remaining.
groupmenu git:(3.0.0-alpha1) curl https://git.drupalcode.org/project/groupmenu/-/merge_requests/6.diff | patch -p1 % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 19680 0 19680 0 0 44509 0 --:--:-- --:--:-- --:--:-- 45874 patching file .gitlab-ci.yml patching file groupmenu.module patching file modules/groupmenu_block/src/Plugin/Block/GroupMenuBlock.php patching file src/Access/GroupMenuAccessInterface.php patching file src/Controller/GroupMenuController.php patching file src/Form/GroupMenuSettingsForm.php patching file src/GroupMenuConfigOverrides.php patching file src/GroupMenuContentListBuilder.php patching file src/GroupMenuListBuilder.php patching file src/GroupMenuService.php patching file src/GroupMenuServiceInterface.php patching file src/Plugin/GroupContentEnabler/GroupMenu.php patching file src/Plugin/GroupContentEnabler/GroupMenuDeriver.php โ groupmenu git:(3.0.0-alpha1) โ cd .. โ contrib git:(master) โ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig groupmenu โ contrib git:(master) โ
Will now move this to RTBC
Thanks,
Jake- ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น