Add API documentation. Make sure that configuration parameters and working with translations are included. Add suitable examples too.

See the documentation in the process plugins for format examples.

CommentFileSizeAuthor
#10 baa55df03342db29aea486974e8f6c3cb7322e6b859863a2f3e29388450e85a8.jpg30.99 KBphenaproxima
#9 2862659-9.patch1.44 KBjofitz
#9 interdiff-6-9.txt1.42 KBjofitz
#6 add_documentation_to-2862659-6.patch1.53 KByogeshmpawar
#6 interdiff-2862659-3-6.txt1.44 KByogeshmpawar
#3 2862659-3.patch1.46 KBjofitz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Parent issue: #2862655: Add documentation to Config destination plugin » #2855895: [meta] Add destination plugin documentation to the codebase
jofitz’s picture

jofitz CreditAttribution: jofitz at ComputerMinds commented
Status: Active » Needs review
FileSize
2862659-3.patch1.46 KB

This one is a bit tricky, but here's a start. I have added:

  • available configuration key(s)
  • a simple example
  • a second example incorporating the translation config key

I couldn't identify destination properties the same as I did with #2862655: Add documentation to Config destination plugin and #2862751: Add documentation to PerComponentEntityDisplay destination plugin.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Assigned: Unassigned » phenaproxima

Self-assigning for review.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Assigned: phenaproxima » Unassigned
Status: Needs review » Needs work

Good start!

  1. +++ b/core/modules/migrate/src/Plugin/migrate/destination/Entity.php
@@ -13,6 +13,48 @@
  * Provides entity destination plugin.

    Can this say "Provides a generic destination to import entities", or something else a little more illuminating? 

  2. +++ b/core/modules/migrate/src/Plugin/migrate/destination/Entity.php
@@ -13,6 +13,48 @@
+ * Available configuration keys:
+ * - translations: (optional) Boolean, if TRUE, the destination is for
+ *   translations. Defaults to FALSE.

    I think we'll need to expand quite a bit on this. Also, what happens if the entity type being imported does not support translations? 

  3. +++ b/core/modules/migrate/src/Plugin/migrate/destination/Entity.php
@@ -13,6 +13,48 @@
+ * This will add the results of the process plugin (for example "nid", "vid" and
+ * "title") to a "node" entity.

    I'd rather rephrase this to "This will save the processed, migrated row as a node." 

  4. +++ b/core/modules/migrate/src/Plugin/migrate/destination/Entity.php
@@ -13,6 +13,48 @@
+ * This will add the results of the process plugin (for example "nid", "vid" and
+ * "title") to a "node" entity associated with the relevant langcode because the
+ * translations configuration is set to "true".

    I'd like to rephrase this similarly.

yogeshmpawar’s picture

yogeshmpawar
He/Him/His
Primary language English
Location 🇮🇳 Pune(MH)
CreditAttribution: yogeshmpawar as a volunteer and at QED42 commented
Status: Needs work » Needs review
FileSize
interdiff-2862659-3-6.txt1.44 KB
add_documentation_to-2862659-6.patch1.53 KB

Changes made as per comment #5.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs review » Needs work

Thanks, @Yogesh Pawar!

+++ b/core/modules/migrate/src/Plugin/migrate/destination/Entity.php
@@ -33,8 +33,8 @@
+ * This will save the processed, migrated row as a node (for example "nid",
+ * "vid" and "title") to a "node" entity.

This phrasing is weird -- it'll save to a node as a node entity? I think we need to streamline this.

I have to leave this NW because not all my feedback from #5 has been addressed.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now 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.

jofitz’s picture

jofitz CreditAttribution: jofitz at ComputerMinds commented
Status: Needs work » Needs review
FileSize
interdiff-6-9.txt1.42 KB
2862659-9.patch1.44 KB

Addressed @phenaproxima's remaining feedback from #5.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs review » Reviewed & tested by the community
FileSize
baa55df03342db29aea486974e8f6c3cb7322e6b859863a2f3e29388450e85a8.jpg30.99 KB

Good!

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented 
+++ b/core/modules/migrate/src/Plugin/migrate/destination/Entity.php
@@ -11,7 +11,48 @@
+ *   plugin: entity:node

Do we want to document anything about the entity:node plugin argument? How it auto finds the generic entity plugin and uses the second part as the entity_type?

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented

Personally, I don't think we need to. There's nothing Migrate-specific about that; it's how derived plugins work. Besides, I think that to Migrate's mainly developer audience, it will be pretty self-explanatory.

  • larowlan committed 2225b0a on 8.5.x 
    Issue #2862659 by Jo Fitzgerald, Yogesh Pawar, phenaproxima: Add...

larowlan credited larowlan.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Reviewed & tested by the community » Fixed

Committed as 2225b0a and pushed to 8.5.x.

Status: Fixed » Closed (fixed)

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