Comments

zaporylie created an issue. See original summary.

zaporylie’s picture

Status: Active » Needs review
StatusFileSize
new4.9 KB

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

borisson_’s picture

There is no phpcs-rule that is added for this, so I'm not sure if we can just rtbc this. I didn't find anything other that uses array(), Array, [], or numeric as @var options.

borisson_’s picture

I thought again about #4 and I think that we should commit this, we can then also fix #2926120: @var tag must not end with a full stop.

And in #2909370: Fix 'Drupal.Commenting.VariableComment.IncorrectVarType' coding standard the phpcs rule can get fixed.

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

per #5.

zaporylie’s picture

Thanks @borisson_

I hope to find time soon to work on mentioned issues.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

@borisson_ but we should add a phpcs rule for this. So we don't regress otherwise we will.

Thank you for your work on cleaning up Drupal core's code style!

In order to fix core coding standards in a maintainable way, all our coding standards issues should be done on a per-rule basis across all of core, rather than fixing standards in individual modules or files. We should also separate fixes where we need to write new documentation from fixes where we need to correct existing standards. This all should be done as part of #2571965: [meta] Fix PHP coding standards in core, stage 1. A good place to start is the child issues of #2572645: [Meta] Fix 'Drupal.Commenting.FunctionComment' coding standard.

For background information on why we usually will not commit coding standards fixes that aren't scoped in that way, see the core issue scope guidelines, especially the note about coding standards cleanups. That document also includes numerous suggestions for scoping issues including documentation coding standards cleanups.

Contributing to the overall plan above will help ensure that your fixes for core's coding standards remain in core the long term.

borisson_’s picture

idebr’s picture

#8/#9 Since this is a child issue out of 3 issues split from #2909370: Fix 'Drupal.Commenting.VariableComment.IncorrectVarType' coding standard I agree it will be difficult to add a sniff covering only this issue.

However, codesniffer found another entry:

core\modules\views_ui\src\ViewUI.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 54 | ERROR | [x] Expected "object" but found "stdClass" for @var tag
    |       |     in member variable comment

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

zaporylie’s picture

Issue tags: +Novice
juliusz_cheddar’s picture

Issue tags: +DrupalEurope

I will work on this issue now.

BartoszUrbaniak’s picture

I will work on this issue now.

zaporylie’s picture

We are working on it together with @Adameue and @BartoszUrbaniak during mentored contribution sprint in Darmstad.

juliusz_cheddar’s picture

Issue tags: +Needs reroll

Needs reroll.
Received this error when trying to apply the patch.
error: core/modules/system/src/Tests/Common/SimpleTestErrorCollectorTest.php: No such file or directory

BartoszUrbaniak’s picture

Issue tags: -Needs reroll
StatusFileSize
new4.94 KB

I have rerolled patch to this issue. Comment #10 is not been addressed.

juliusz_cheddar’s picture

StatusFileSize
new5.37 KB
new445 bytes

Created patch addressed to #10.

juliusz_cheddar’s picture

Status: Needs work » Needs review
BartoszUrbaniak’s picture

Status: Needs review » Reviewed & tested by the community

I have applied 2926122-18.patch.
After this I run this command:
vendor/bin/phpcs --standard=core/phpcs.xml --runtime-set installed_paths vendor/drupal/coder/coder_sniffer -- '-ps'
Results which I received from CodeSniffer scan shows that previous errors has been repaired.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed dd7fd60f19 to 8.7.x and ace96040c4 to 8.6.x. Thanks!

As these are docs fixes and no new coding standard is enforced backported to 8.6.x.

  • alexpott committed dd7fd60 on 8.7.x
    Issue #2926122 by Adameue, zaporylie, BartoszUrbaniak, borisson_, idebr...

  • alexpott committed ace9604 on 8.6.x
    Issue #2926122 by Adameue, zaporylie, BartoszUrbaniak, borisson_, idebr...
zaporylie’s picture

Thank you alexpott!

Status: Fixed » Closed (fixed)

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