Part of #2909364: [meta] Fix 'Drupal.Commenting.VariableComment' coding standard.

See parent issue for instructions.

CommentFileSizeAuthor
#15 fix-2909366-15.patch742 byteszaporylie
#12 fix-2909366-12.patch742 byteszaporylie
#6 2909366-phpcs-drupal-commenting-variablecomment-emptyvar.patch606 bytesiKb

Comments

mfernea created an issue. See original summary.

mfernea’s picture

mfernea commented

Currently there's only 1 issue. In GlossaryViewTest. 

  /**
   * @var
   * The additional language to use.
   */
  protected $language;

Needs some research.

zaporylie’s picture

zaporylie
he/him
Primary language Polish
commented
Issue tags: +Novice, +Vienna2017
iKb’s picture

iKb commented

I am at DrupalCon Vienna 2017 and i'll try to fix this.

iKb’s picture

iKb commented
Assigned: Unassigned » iKb
iKb’s picture

iKb commented
StatusFileSize
new 2909366-phpcs-drupal-commenting-variablecomment-emptyvar.patch606 bytes
iKb’s picture

iKb commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 6: 2909366-phpcs-drupal-commenting-variablecomment-emptyvar.patch, failed testing. View results

reflie’s picture

reflie commented
Assigned: iKb » reflie
zaporylie’s picture

zaporylie
he/him
Primary language Polish
commented
Related issues: +#2914677: Unused member variable in GlossaryViewTest

@reflie - before you start working on this please look at #2914677: Unused member variable in GlossaryViewTest first

reflie’s picture

reflie commented
Assigned: reflie » Unassigned
zaporylie’s picture

zaporylie
he/him
Primary language Polish
commented
StatusFileSize
new fix-2909366-12.patch742 bytes

This is actually super-novice issue now, when #2914677: Unused member variable in GlossaryViewTest got committed.

zaporylie’s picture

zaporylie
he/him
Primary language Polish
commented
Status: Needs work » Needs review

Forgot to change a status.

mfernea’s picture

mfernea commented
Status: Needs review » Needs work 
+++ b/core/phpcs.xml.dist
@@ -64,8 +64,7 @@
+    <!-- Sniff for: DuplicateVar, InlineVariableName, EmptyVar -->

Let's keep the list in alphabetical order.

zaporylie’s picture

zaporylie
he/him
Primary language Polish
commented
Status: Needs work » Needs review
StatusFileSize
new fix-2909366-15.patch742 bytes

Good point. I'm adding only patch - no point for having interdiff here :)

mfernea’s picture

mfernea commented
Status: Needs review » Reviewed & tested by the community

Looks good now! :)

  • xjm committed 431e90f on 8.5.x 
    Issue #2909366 by zaporylie, iKb, mfernea: Fix 'Drupal.Commenting....
xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Ran PHPCS against 8.5.x and confirmed that the rule passes as-is. I also did this:

diff --git a/core/lib/Drupal.php b/core/lib/Drupal.php
index 07dc12f1f3..e97cadbd74 100644
--- a/core/lib/Drupal.php
+++ b/core/lib/Drupal.php
@@ -78,6 +78,11 @@
  */
 class Drupal {
 
+  /**
+   * @var
+   */
+  protected $foo;
+
   /**
    * The current system version.
    */

And confirmed that the rule would fail in that case.

Committed to 8.5.x. Thanks! I didn't backport it to 8.4.x since it's only a rule addition.

Status: Fixed » Closed (fixed)

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