PluginFormInterface contains documentation that is specific to \Drupal\Component\Plugin\PluginBase and that has nothing to do with the interface itself.

CommentFileSizeAuthor
#7 drupal_2252081_7.patch1.63 KBXano
#1 drupal_2252081_1.patch725 bytesXano
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented
Status: Active » Needs review
FileSize
drupal_2252081_1.patch725 bytes
jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Needs work

Do you think that documentation should be moved elsewhere, such as a base class method that people might actually be overriding? And... yeah it's an implementation-specific doc, but it's really just a suggestion for people who are implementing the method. It seems helpful... We should have it somewhere.

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented

First thought, and second: no. The reason we have interfaces is that we do not care about implementations at all. If someone works with PluginBase and they want to implement PluginFormInterface without knowing how PluginBase stores its confirmation, then that is not our problem. We cannot-and should not-add suggestions to our interface, because 1) we can keep doing that until the end of time and still not be done and 2) this has brought us more bad than good in the past, where we kept coding until only suggested usages were supported.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

Fine, then can we move this suggestion to the base class then, assuming it's a hint for module developers?

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented

We could. Do you have a clear way of doing this in mind? There are a number of interfaces that deal with configuration (ConfigurablePluginInterface being another), and PluginBase already has an in-line code comment about $this->configuration. If you don't, no wories. I'll work something out and post a patch for review.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

People aren't probably going to read in-line comments as likely as doc blocks. So probably this belongs on the PluginBase in a doc block somewhere... or else maybe we can just get rid of it. It just seemed like it was useful information that a plugin developer might need to have?

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented
Status: Needs work » Needs review
FileSize
drupal_2252081_7.patch1.63 KB

What about this? PluginFormInterface is in Core, so we shouldn't really be referencing that from Component.

jhodgdon’s picture

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

That looks reasonable to me. I'll mark it RTBC and let it sit for a few days for others to review/object/offer suggestions. Thanks!

  • Commit ef78dcc on 8.x by jhodgdon: 
    Issue #2252081 by Xano: Move implementation-specific docs out of...
jhodgdon’s picture

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

Thanks again! Committed to 8.x.

Status: Fixed » Closed (fixed)

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