Problem/Motivation

The documentation is wrong for the namespace in few field formatter plugins. Ex- responsive images

Proposed resolution

Fix it

Remaining tasks

User interface changes

API changes

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug
Issue priority Minor, since it's only a small documentation flaw
Unfrozen changes Unfrozen because it only changes documentation.
CommentFileSizeAuthor
#15 wrong_documentation_for-2494551-15.patch732 bytesNitebreed
#5 wrong_file-2494551-5.patch11.17 KBgoogletorp
#5 interdiff.txt7.48 KBgoogletorp
#4 2494551-4.patch3.69 KBjoshi.rohit100
#1 wrong_file-2494551-1.patch704 bytesgoogletorp
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

googletorp’s picture

googletorp CreditAttribution: googletorp at Reveal IT commented
Status: Active » Needs review
FileSize
wrong_file-2494551-1.patch704 bytes

Uploaded patch.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented

I can see the same documentation problem with few other modules as well, name few - link, telephone, text..
So I think IS needs an update to accomodate these as well.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented
Title: Wrong @file documentation for ResponsiveImageFormatter.php » Wrong @file documentation for field formatter plugins.
Issue summary: View changes
joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented
FileSize
2494551-4.patch3.69 KB
googletorp’s picture

googletorp CreditAttribution: googletorp at Reveal IT commented
FileSize
interdiff.txt7.48 KB
wrong_file-2494551-5.patch11.17 KB

I didn't notice it was a general problem, added some more cases, you didn't find, I think we got them all now.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Status: Needs review » Needs work
Issue tags: +Needs issue summary update

Is there a way to be sure you have found all of them besides a careful visual inspection?

+++ b/core/modules/options/src/Tests/OptionsFormattersTest.php
@@ -11,8 +11,8 @@
  * Tests the Options field type formatters.
  *
  * @group options
- * @see \Drupal\options\Plugin\field\formatter\OptionsDefaultFormatter
- * @see \Drupal\options\Plugin\field\formatter\OptionsKeyFormatter
+ * @see \Drupal\options\Plugin\Field\FieldFormatter\OptionsDefaultFormatter
+ * @see \Drupal\options\Plugin\Field\FieldFormatter\OptionsKeyFormatter
  */
 class OptionsFormattersTest extends OptionsFieldUnitTestBase {

This is not a @file comment. Maybe the issue needs retitling.

googletorp’s picture

googletorp CreditAttribution: googletorp at Reveal IT commented
Title: Wrong @file documentation for field formatter plugins. » Wrong documentation for field formatter plugins namespaces.
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update
googletorp’s picture

googletorp CreditAttribution: googletorp at Reveal IT commented
Issue tags: -Quick fix

Probably not quick fix anymore.

cilefen’s picture

cilefen CreditAttribution: cilefen commented

Something like this could give you an idea how many could be wrong in the modules:

egrep -r '\*.*\\Drupal.*[pP]lugin' core/modules/*/src/Plugin/

cilefen’s picture

cilefen CreditAttribution: cilefen commented

Or, you could change the issue scope back to the original @file comments and focus on them.

Nitebreed’s picture

Nitebreed
Primary language Dutch
CreditAttribution: Nitebreed commented

I'm working on this issue currently at DrupalCon Barcelona together with legolasbo as my mentor.

Nitebreed queued 5: wrong_file-2494551-5.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 5: wrong_file-2494551-5.patch, failed testing.

attiks’s picture

attiks CreditAttribution: attiks at Attiks commented
Component: responsive_image.module » field system

Changed the component, since it affects multiple modules.

Nitebreed’s picture

Nitebreed
Primary language Dutch
CreditAttribution: Nitebreed commented
Status: Needs work » Needs review
FileSize
wrong_documentation_for-2494551-15.patch732 bytes

Because the last patch didn't apply anymore, I created a new patch that takes care of the places where the wrong namespace still existed.

oenie’s picture

oenie CreditAttribution: oenie commented
Status: Needs review » Needs work

It seems to me your reroll has left out a lot of changes from the patch you have rerolled.
Is that what you wanted or has it been split ?

Nitebreed’s picture

Nitebreed
Primary language Dutch
CreditAttribution: Nitebreed commented

No, those changes were already added in between the last 4 months. So the only ones left were added to the patch.

googletorp’s picture

googletorp CreditAttribution: googletorp at Reveal IT commented
Status: Needs work » Reviewed & tested by the community

This looks good.

A lot of the other work that was in the patch, was solved in a big fix all @file class name docs that was committed a month ago or so.

This patch addresses the rest of the incorrect documentation containing "...\field\formatter...".

So all good and RTBC imo.

googletorp’s picture

googletorp CreditAttribution: googletorp at Reveal IT commented
googletorp’s picture

googletorp CreditAttribution: googletorp at Reveal IT commented
Related issues: +#1392754: Comply with new documentation standards for @file for namespaced class files

Add related issue, that I was talking about in #18.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed 872ef36 and pushed to 8.0.x. Thanks!

  • alexpott committed 872ef36 on 8.0.x 
    Issue #2494551 by googletorp, joshi.rohit100, Nitebreed, cilefen: Wrong...

Status: Fixed » Closed (fixed)

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