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.

Actually, as pointed out in #12, the Empty source plugin does not have anything to do with translations. So, that is not a consideration here.

CommentFileSizeAuthor
#10 interdiff-2862673-5-9.txt980 bytesmasipila
#10 2862673-9.patch888 bytesmasipila
#5 interdifff.txt528 bytesPavan B S
#5 2862673-5.patch928 bytesPavan B S
#2 2862673-2.patch819 bytesjofitz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

jofitz’s picture

jofitz CreditAttribution: jofitz at ComputerMinds commented
Status: Active » Needs review
FileSize
2862673-2.patch819 bytes

This is a start, at least. I don't believe there are any configuration keys for this source plugin.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/migrate/src/Plugin/migrate/source/EmptySource.php
@@ -5,7 +5,20 @@
  * Source returning an empty row.
  *
- * This is generally useful when needing to create a field using a migration..
+ * This is generally useful when needing to create a field using a migration.

Hmm... but is doesn't return an empty row? It returns a row based on the constants provided right?

Pavan B S’s picture

Pavan B S CreditAttribution: Pavan B S at Valuebound commented
FileSize
2862673-5.patch928 bytes
interdifff.txt528 bytes

Applying the patch based on comment suggested in #4

boaloysius’s picture

boaloysius CreditAttribution: boaloysius as a volunteer and at Google Summer of Code commented
Status: Needs work » Needs review
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
Status: Needs review » Needs work 
  1. +++ b/core/modules/migrate/src/Plugin/migrate/source/EmptySource.php
@@ -3,9 +3,22 @@
+ * This is generally useful when needing to create a field using a migration.

    I would strike this sentence entirely. It's a Migrate Drupal example, and this is a Migrate plugin (i.e., lower level than Migrate Drupal). 

  2. +++ b/core/modules/migrate/src/Plugin/migrate/source/EmptySource.php
@@ -3,9 +3,22 @@
+ * This will enable creation of data, based on the "constants", without an
+ * existing source.
  *

    This paragraph is confusing. Can it simply explain the example: "This will return a single row containing 'entity_type' and 'field_name' elements, with values of 'user' and 'image', respectively."

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.

masipila’s picture

masipila CreditAttribution: masipila as a volunteer commented
Assigned: phenaproxima » Unassigned
Status: Needs work » Needs review
FileSize
2862673-9.patch888 bytes
interdiff-2862673-5-9.txt980 bytes

This patch should address the remaining topics from #8.

@phenaproxima, this old issue has been assigned to you for the past 7 months :) I unassigned you so that others can review this as well. Feel free to assign yourself back to this if you want to review this by yourself.

Cheers,
Markus

p.s. Both of the remaining issues of this parent meta are now in 'Needs review', yay :)

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +API Documentation

Nice, concise, and eminently useful. This is my kind of patch. Thanks a million, @masipila!

RTBC once Drupal CI passes it.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Reviewed & tested by the community » Needs review

Hm, issue summary says "Make sure that configuration parameters and working with translations are included.". Is that some common issue text used? Not sure what would apply here for translations.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Issue summary: View changes

Yes, this plugin does not need to address translations. The text in the issue summary was boilerplate when making these documentation issues to ensure that translations were considered. And it worked! We now explicitly state that nothing needs to be done for translations.

  • Gábor Hojtsy committed 0f64348 on 8.5.x 
    Issue #2862673 by masipila, Pavan B S, Jo Fitzgerald, phenaproxima: Add...
quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Oh, forgot to RTBC

  • Gábor Hojtsy committed aeb3efa on 8.4.x 
    Issue #2862673 by masipila, Pavan B S, Jo Fitzgerald, phenaproxima: Add...
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Version: 8.5.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Fixed

Looks good to me then. Thanks all!

Status: Fixed » Closed (fixed)

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