Support from Acquia helps fund testing for Drupal Acquia logo

Comments

attiks created an issue. See original summary.

DuaelFr’s picture

Issue tags: -Novice

As agreed between the mentors at Drupalcon, according to issues to avoid for novices, I am untagging this issue as "Beginner". This issue contains changes across a very wide range of files and might create too many other patches to need to be rerolled at this particular time. This patch has an automated way to be rerolled later so better to implement it after Drupalcon.

tatarbj’s picture

I've used this command to check the sniff on the core:
phpcs --standard=Drupal --sniffs=Drupal.Commenting.HookComment --ignore=vendor,assets/vendor core/
the result was:

FILE: ...system/tests/modules/module_test/module_test.implementations.inc
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 4 | WARNING | Format should be "* Implements hook_foo().", "*
   |         | Implements hook_foo_BAR_ID_bar() for xyz_bar().",, "*
   |         | Implements hook_foo_BAR_ID_bar() for
   |         | xyz-bar.html.twig.", or "* Implements
   |         | hook_foo_BAR_ID_bar() for xyz-bar.tpl.php.".
----------------------------------------------------------------------

Attaching the patch here what solve it, other issues do not exists on the core now.

attiks’s picture

Status: Needs review » Reviewed & tested by the community

Thanks

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Test code change - permitted in beta. Committed 93cd09f and pushed to 8.0.x. Thanks!

  • alexpott committed 93cd09f on 8.0.x
    Issue #2572649 by tatarbj: Fix 'Drupal.Commenting.HookComment' coding...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

alexpott’s picture

This was not really fixed... there is well over 100+ in core... :( and they have not been introduced since. Maybe the sniff has changed. We need a followup issue to fix the rest of these and remove the exclusion from phpcs.xml.dist

attiks’s picture