Problem/Motivation

\Drupal still has an @file docblock. Our coding standards were updated to remove these with 8.1.x.

According to @alexpott, the reason our coding standards checks (as well as the original conversion) have not picked this up is that the class has no namespace and we haven't ever solved how to get the rule to work for that.

Proposed resolution

Remove the docblock manually.

Update ApI documentation, @file: Documenting files to:

"The @file doc block MUST be present for all PHP files, with two exceptions: 1) the Drupal class file, and 2) files that contain a namespaced class/interface/trait, whose file name is the class name with a .php extension, and whose file path is closely related to the namespace (under PSR-4 or a similar standard), SHOULD NOT have a @file documentation block. "

Remaining tasks

Review
Commit

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm created an issue. See original summary.

xjm’s picture

Issue tags: +Novice, +Coding standards
cilefen’s picture

Status: Needs review » Reviewed & tested by the community

How did we miss that?

catch’s picture

Status: Reviewed & tested by the community » Needs work

This actually fails Drupal CS because the class has no namespace, so can't be committed as is.

----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 1 | ERROR | [x] Missing file doc comment
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
xjm’s picture

Status: Needs work » Postponed

lol. So it's not just that the fixer did not fix it; it's that Coder actually fails when it's correct. I looked for a Coder issue and did not find one, just #2828946: Still giving me warnings about file doc comments?.

I guess this is postponed on making Coder support it, though it might not be worth the effort.

xjm’s picture

Issue tags: -Novice
catch’s picture

It's not just that, because the class isn't namespaced, the coding standards say it must have a @file docblock. https://www.drupal.org/docs/develop/coding-standards/api-documentation-a...

So really to change it, we need to relax the @file requirements in the coding standards - i.e. include any class where there's one file per class?

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

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

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should 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.

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

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should 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.

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

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

Version: 8.9.x-dev » 9.2.x-dev
Status: Postponed » Needs review
Issue tags: +Bug Smash Initiative
FileSize
349 bytes

I wonder if anything has changed. Rerolled the patch.

Kristen Pol’s picture

Thanks for the reroll.

1) Tests pass now.

2) Per #7, there was a question of whether it really should be removed. According to the docs:

The @file doc block MUST be present for all PHP files, with one exception: files that contain a namespaced class/interface/trait, whose file name is the class name with a .php extension, and whose file path is closely related to the namespace (under PSR-4 or a similar standard), SHOULD NOT have a @file documentation block.

So, now that the tests pass, if we want to keep this change, the documentation needs to be updated to reflect this change, since it only refers to namespaced class/interface/trait. Here's a version with the Drupal class file explicitly referenced:

The @file doc block MUST be present for all PHP files, with two exceptions: 1) the Drupal class file, and 2) files that contain a namespaced class/interface/trait, whose file name is the class name with a .php extension, and whose file path is closely related to the namespace (under PSR-4 or a similar standard), SHOULD NOT have a @file documentation block.

Kristen Pol’s picture

quietone’s picture

Issue summary: View changes

@Kristen Pol, thanks yes the docs need to be updated.

jhodgdon’s picture

Can we discuss the docs wording a bit? Here's my suggestion:

The @file doc block MUST be present for PHP files, with the following exceptions:
<ul>
<li>Files that contain a namespaced class/interface/trait, whose file name is the class name with a .php extension, and whose file path is closely related to the namespace (under PSR-4 or a similar standard), SHOULD NOT have a @file documentation block.</li>
<li>The core \Drupal class's Drupal.php file (which does not have a namespace).</li>
</ul>
Kristen Pol’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @jhodgdon. Looks good to me. I'll mark this RTBC as the docs change can happen when this gets committed.

jhodgdon’s picture

I'll go ahead and update the page now. I don't think it's a problem to do it first.

  • catch committed 1c1dbe0 on 9.2.x
    Issue #2829453 by xjm, quietone, Kristen Pol, catch, jhodgdon: \Drupal...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Nice to see this can go in now.

Committed 1c1dbe0 and pushed to 9.2.x. Thanks!

Status: Fixed » Closed (fixed)

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