Problem/Motivation

There are some Schema functions that don't have the doc comment in the concrete classes but the documentation is in the abstract class \Drupal\Core\Database\Schema.

Proposed resolution

Add the 

  /**
   * {@inheritdoc}
   */

bloc comment to the functions

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#7 add_inheritdoc_function_doc_comment_to_schema_classes-2863750-2.patch17.27 KBtstoeckler
#2 add_inheritdoc_function_doc_comment_to_schema_classes-2863750-2.patch17.27 KBadriancid
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

adriancid created an issue. See original summary.

adriancid’s picture

adriancid
Primary language Spanish
Location Montreal, Canada
CreditAttribution: adriancid as a volunteer and commented
Status: Active » Needs review
FileSize
add_inheritdoc_function_doc_comment_to_schema_classes-2863750-2.patch17.27 KB
joe_carvajal’s picture

joe_carvajal
He/him
Primary language Spanish
Location Seville (Spain)
CreditAttribution: joe_carvajal at La Drupalera by Emergya commented
Status: Needs review » Reviewed & tested by the community

Seems to be OK. This way these methods can inherit the doc from Drupal\Core\Database\Schema.

Thank you for reporting it!

joe_carvajal’s picture

joe_carvajal
He/him
Primary language Spanish
Location Seville (Spain)
CreditAttribution: joe_carvajal at La Drupalera by Emergya commented
Issue tags: +DevDaysSeville
daffie’s picture

daffie CreditAttribution: daffie commented

+1 RTBC

+++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php
@@ -471,6 +470,9 @@ public function tableExists($table) {
+   */  ¶

Can be removed on commit: 2 trailing whitespaces.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Issue summary: View changes
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler as a volunteer commented
Status: Reviewed & tested by the community » Needs review
FileSize
add_inheritdoc_function_doc_comment_to_schema_classes-2863750-2.patch17.27 KB

Literally removed the spaces from the patch file. Seems unnecessary to bother committers with this for a docs patch.

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.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Status: Needs review » Reviewed & tested by the community

Re-RTBCing for the nitfix

  • xjm committed fdf6bf9 on 8.5.x 
    Issue #2863750 by adriancid, tstoeckler: Add inheritdoc function doc...

  • xjm committed 6472299 on 8.4.x 
    Issue #2863750 by adriancid, tstoeckler: Add inheritdoc function doc...
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Fixed
Related issues: +#2571965: [meta] Fix PHP coding standards in core

So, normally, we require fixing a single rule or sub-rule across all of core in a single issue. However, fixing Drupal.Commenting.ClassComment.Missing is going to be massive (#2571965: [meta] Fix PHP coding standards in core says there were 161 of them a year-and-some ago), and in many cases adding new documentation requires actually writing documentation. In this case we have a lot of known offenders concentrated in one namespace with the same fix pretty much everywhere in the namespace, so I think it makes sense to fix these as scoped. All we have to do is confirm that these methods do exist on the parent class and are being inherited.

Committed to 8.5.x, and cherry-picked to 8.4.x as a documentation addition. Thanks!

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented

@adriancid congrats on your first core commit credit!

adriancid’s picture

adriancid
Primary language Spanish
Location Montreal, Canada
CreditAttribution: adriancid as a volunteer and commented

Thanks @tim.plunkett I hope that this is just the begining :-D

Status: Fixed » Closed (fixed)

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