Here's a patch that adds missing documentation to \Drupal\feeds\Plugin\Type\Target\TargetInterface

Do you think setTarget could also be updated to take FieldTargetDefinition instead of EntityInterface or does it do unnecessary code breaks?

CommentFileSizeAuthor
#7 target-interface-docs-2960318-7.patch1.14 KBmikran
#4 target-interface-docs-2960318-3.patch1.13 KBmikran
#3 interdiff-2960318.txt564 bytesmikran
#2 target-interface-docs-2960318-2.patch564 bytesmikran
#2 interdiff-2960318.txt564 bytesmikran
target-interface-docs.patch1.12 KBmikran

Comments

mikran created an issue. See original summary.

mikran’s picture

mikran commented
StatusFileSize
new interdiff-2960318.txt564 bytes
new target-interface-docs-2960318-2.patch564 bytes

Small correction to previous patch.

mikran’s picture

mikran commented
StatusFileSize
new interdiff-2960318.txt564 bytes

Oops, correct patch

mikran’s picture

mikran commented
StatusFileSize
new target-interface-docs-2960318-3.patch1.13 KB

...

mikran’s picture

mikran commented
megachriz’s picture

megachriz
Primary language Dutch
commented

Thanks for working on this!

+++ b/src/Plugin/Type/Target/TargetInterface.php
@@ -14,17 +14,24 @@ interface TargetInterface {
+   * @param \Drupal\feeds\FieldTargetDefinition[] $targets

Quick review: a target does not have to be a field target. That's why there is a TargetBase class separate from the FieldTargetBase class (as far as I know though there isn't yet a target that does not extend FieldTargetBase). I think the parameter type should be \Drupal\feeds\TargetDefinition instead, though I'm not completely sure.

mikran’s picture

mikran commented
StatusFileSize
new target-interface-docs-2960318-7.patch1.14 KB

Oh that is true. I've now changed it to be \Drupal\feeds\TargetDefinitionInterface[] instead.

megachriz’s picture

megachriz
Primary language Dutch
commented 
  1. +++ b/src/Plugin/Type/Target/TargetInterface.php
@@ -14,17 +14,24 @@ interface TargetInterface {
+   * @return \Drupal\feeds\TargetDefinitionInterface[]
+   *   An array of targets.

    At least FieldTargetBase does not return targets in all cases. 

  2. +++ b/src/Plugin/Type/Target/TargetInterface.php
@@ -14,17 +14,24 @@ interface TargetInterface {
+   * @param \Drupal\Core\Entity\EntityInterface $target_item

    Maybe rename $target_item to $entity? That's what the implementation in FieldTargetBase does.

  • MegaChriz committed 423b2ef on 8.x-3.x authored by mikran 
    Issue #2960318 by mikran, MegaChriz: Added docs for \Drupal\feeds\Plugin...
megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Needs review » Fixed

Committed #7 with the changes suggested in #8.

Status: Fixed » Closed (fixed)

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