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
#7 2862672-7.patch1.46 KBjofitz
#7 interdiff-2-7.txt967 bytesjofitz
#5 2862672-5.patch707 bytesjofitz
#5 interdiff-2-5.txt707 bytesjofitz
#2 2862672-2.patch1.44 KBquietone

Comments

quietone created an issue. See original summary.

quietone’s picture

quietone commented
Status: Active » Needs review
StatusFileSize
new 2862672-2.patch1.44 KB

A start. Took the example from the original issue for creating this plugin.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Assigned: Unassigned » heddn

Assigning to myself for review this week.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Assigned: heddn » Unassigned
Status: Needs review » Needs work 
+++ b/core/modules/migrate/src/Plugin/migrate/source/EmbeddedDataSource.php
@@ -1,10 +1,41 @@
+ * Allows source data to be defined in the source plugin.

I'm not happy with how this sounds. We don't source the code from the plugin itself; the php doesn't provide the source data.

The configuration of the the plugin provides the data. I'm not happy with how this sounds, but here's an attempt.

"Provides source data from the configuration of the source plugin.

jofitz’s picture

jofitz
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2-5.txt707 bytes
new 2862672-5.patch707 bytes

I've gone for the best of both suggestions:

"Allows source data to be defined in the configuration of the source plugin."

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Needs work 
+++ b/core/modules/migrate/src/Plugin/migrate/source/EmbeddedDataSource.php
@@ -5,7 +5,7 @@
  * The embedded_data source plugin is used to inject source data directly in the
  * plugin itself. One use case is when some small amount of fixed data is

This could use some attention then too. It has the same wording, logic as previously.

jofitz’s picture

jofitz
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2-7.txt967 bytes
new 2862672-7.patch1.46 KB

Let's pretend #5 never happened...

This patch addresses the comments in #4 and #6.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Reviewed & tested by the community

Love it. LGTM.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: 2862672-7.patch, failed testing.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs work » Reviewed & tested by the community

Random testbot failure.

  • Gábor Hojtsy committed 164864e on 8.3.x 
    Issue #2862672 by Jo Fitzgerald, quietone, heddn: Add documentation to...

  • Gábor Hojtsy committed af7d608 on 8.4.x 
    Issue #2862672 by Jo Fitzgerald, quietone, heddn: Add documentation to...
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Version: 8.4.x-dev » 8.3.x-dev
Status: Reviewed & tested by the community » Fixed

Looks good, committed :)

Status: Fixed » Closed (fixed)

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