The D8 documentation standards at https://www.drupal.org/node/1354#inheritdoc call for {@inheritdoc} (including curly braces) to be the only text in a doc block. Attached is a patch correcting about ten invalid formats that currently exist within core.

CommentFileSizeAuthor
#11 2606724-11-inheritdoc-format-fix-d8.patch11.89 KBLars Toomre
#11 2606724-11-1-interdiff.txt6.53 KBLars Toomre
#4 interdiff-2606724-2-3.txt1.99 KBimalabya
#3 2606724-2.patch7.34 KBimalabya
inheritdoc-format-fix-d8.patch5.36 KBLars Toomre
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Lars Toomre created an issue. See original summary.

Lars Toomre’s picture

Lars Toomre CreditAttribution: Lars Toomre commented
Category: Task » Bug report

Appears these type of documentation fixes should be classified as bug fixes.

imalabya’s picture

imalabya
he/him
Primary language English
Location Bangalore
CreditAttribution: imalabya at Valuebound commented
FileSize
2606724-2.patch7.34 KB

Hi,
Found a few more instances of the case. Have added the patch and interdiff file.

imalabya’s picture

imalabya
he/him
Primary language English
Location Bangalore
CreditAttribution: imalabya at Valuebound commented
FileSize
interdiff-2606724-2-3.txt1.99 KB

The interdiff file got missed.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +rc eligible

Thanks!

Lars Toomre’s picture

Lars Toomre CreditAttribution: Lars Toomre commented
Status: Reviewed & tested by the community » Needs work

Thanks @malavya for rolling a patch with additional {@inheritdoc} changes. Unfortunately the three changes you added are subtly wrong. Your additions caught the @inheritDoc errors (with a capital 'D'). The proper string is '{@inheritdoc}' (with a lower case 'd'). Hence I am setting back to 'Needs work.'

Checking the core code base, I see that there are approximately ten additional files that need to be changed from '{@inheritDoc}' to the documentation standard of '{@inheritdoc}'. I am unsure of how many files we should include in a simple documentation patch like this before it becomes too big or disruptive. That is why I was targeting about ten files in the initial patch. Perhaps one of the core committers can give some feedback?

Lars Toomre’s picture

Lars Toomre CreditAttribution: Lars Toomre commented

@jodgdon: If you see this comment, perhaps you can also advise if '@inheritdoc' is permissible in our javascript documentation standards. Both of the patches in this issue only address *.php files. There are approximately a dozen *.js files that might need to be addressed as well.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

Thanks Lars, you are right. Let's get all the inheritdoc problems together.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

Regarding #7, the person to ask about that would be _nod, the JavaScript maintainer. I believe there is a standards page for JavaScript docs too. Not my expertise, sorry!

The last submitted patch, inheritdoc-format-fix-d8.patch, failed testing.

Lars Toomre’s picture

Lars Toomre CreditAttribution: Lars Toomre commented
Status: Needs work » Needs review
FileSize
2606724-11-1-interdiff.txt6.53 KB
2606724-11-inheritdoc-format-fix-d8.patch11.89 KB

Here is an updated patch taking into account both comments #6 and #7. The attached interdiff is against the original patch since the changes in #3 were slightly wrong (capital 'D' vis-a-vis lower case 'd'). Thanks for your help with this issue @malavya.

imalabya’s picture

imalabya
he/him
Primary language English
Location Bangalore
CreditAttribution: imalabya at Valuebound commented

I checked into the documentation for the @inheritdoc for JavaScript
https://developers.google.com/closure/compiler/docs/js-for-compiler?hl=en

The documentation shows the example as:

/** @inheritDoc */
project.SubClass.prototype.toString = function() {
  ...
};

Currently we have:

/**
     * @inheritdoc
     */
    render: function () {
    },

@jhodgdon, @_nod what do you suggest? should we be changing it as shown in the documentation?

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer commented

I'd rather leave is alone. we're not following google code style. Also keeping the current header makes things consistent, which is nice.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -rc eligible

Thanks for the patch! Sorry for delay in review -- I've been on vacation.

So the patch in #11 seems fine to me, and we should not be changing JS files, so let's just get this in.

jhodgdon’s picture

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

Thanks again! Committed to both 8.x branches.

  • jhodgdon committed 0fb0add on 8.1.x 
    Issue #2606724 by Lars Toomre, malavya: Few {@inheritdoc} formatting...

Status: Fixed » Closed (fixed)

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