Improve description of key concepts in migrate.api.php documentation

Magnificent work. I only request a few minor verbiage changes, then this is good to go.

1. +++ b/core/modules/migrate/migrate.api.php
   @@ -82,6 +79,39 @@
   + * parent term has not yet been migrated. Migrate API addresses this 'chicken or
   + * the egg' dilemma by creating a stub term for the parent so that the child
   

   Should be 'chicken and egg'.

2. +++ b/core/modules/migrate/migrate.api.php
   @@ -82,6 +79,39 @@
   + * source ID and the hash in the map allow for tracking changes for continuous
   

   Let's remove 'in the map'.

3. +++ b/core/modules/migrate/migrate.api.php
   @@ -82,6 +79,39 @@
   + * establishing relations between records.
   

   'relations' should be 'relationships'.

4. +++ b/core/modules/migrate/migrate.api.php
   @@ -82,6 +79,39 @@
   + * only content that has been created or updated in the source since the
   

   Let's change 'content' to 'data'.

5. +++ b/core/modules/migrate/migrate.api.php
   @@ -82,6 +79,39 @@
   + * migrated so far. If the source system has for example a timestamp that
   + * indicates when a node was created or last updated, that timestamp could be
   + * used as a highwater property. If we execute the migration again, only those
   

   Let's rephrase this whole sentence: "For example, a timestamp property that indicates when a row of data was created or last updated would make an excellent highwater property."

6. +++ b/core/modules/migrate/migrate.api.php
   @@ -82,6 +79,39 @@
   + * nodes that have a higher timestamp than in the previous migration would be
   

   'nodes' should be 'rows'; this documentation doesn't need to be Drupal-specific.

7. +++ b/core/modules/migrate/migrate.api.php
   @@ -82,6 +79,39 @@
   + * It is quite typical that when developing a migration, the first version does
   

   Let's rephrase: "When developing a migration, it is quite typical..."

8. +++ b/core/modules/migrate/migrate.api.php
   @@ -82,6 +79,39 @@
   + * migration, adjust the migration and then execute it again.
   

   Can this be slightly rephrased? "...undo a migration, then execute it again after adjusting it."

1. +++ b/core/modules/migrate/migrate.api.php
   @@ -82,6 +79,39 @@
   + * parent term has not yet been migrated. Migrate API addresses this 'chicken or
   + * the egg' dilemma by creating a stub term for the parent so that the child
   

   This is more typically called "Chicken or egg" in English. Ahh, I see that @phenaproxima saw the same thing.

2. +++ b/core/modules/migrate/migrate.api.php
   @@ -82,6 +79,39 @@
   + * term can establish a reference to it. When the parent term is eventually
   + * being migrated, Migrate API updates the previously created stub with the
   

   is eventually migrated. Loose the word "being", I think it will read smoother.

+++ b/core/modules/migrate/migrate.api.php
@@ -82,6 +79,38 @@
+ * Once a migrated row is saved and the destination ID is known, Migrate API
+ * saves the source ID, destination ID and the row hash into a map table. The
+ * source ID and the hash allow tracking changes for continuous migrations.

There might be more than one source or destination id. So we need to account for plurality. I think this is important to document as folks get confused and think they can only use a single id. Which isn't correct, multiple are supported.

Using plurals as suggested in #6.

Also change a sentence in the highwater section to remove use of 'we'.

Just one question

+++ b/core/modules/migrate/migrate.api.php
@@ -82,6 +79,38 @@
+ * so far. For example, a timestamp property that indicates when a row of data
+ * was created or last updated would make an excellent highwater property. If we

How can one set the current timestamp as hwm in the migration template? Maybe we can add a code example here.

Wow, that question would never occur to me! I don't think a code example is needed because the only thing the user needs to do is declare the high_water_property. I have reworded that section, hoping to emphasize that point.

The rewording looks good now. About the example, I had to refresh my memory about hwm usage by reading \Drupal\Tests\migrate\Kernel\HighWaterTest and high_water_test.yml. :D

I think we can say something like, e.g. for D7 node migration 'changed' source property can be used to set up the highwater mark

source:
  plugin: d7_node
  high_water_property:
    name: changed

Example added.

+++ b/core/modules/migrate/migrate.api.php
@@ -107,6 +107,15 @@
+ *   plugin: source_plugin
...
+ *     name: changed

Without the source plugin 'd7_node' 'changed' is missing the context here.

+++ b/core/modules/migrate/migrate.api.php
@@ -82,6 +79,48 @@
+ * In this example, the row property 'changed' is the high_water_property.

How about
In this example, the row property 'changed' is the high_water_property. The row property 'changed' contains the last updated timestamp for each row which will be used to setup highwater mark.
or something along these lines.

Thanks. I did try something like that extra explanation but failed (have a headache which is distracting) but I could work with your suggestions and do something useful.

Thank you for updating the patch. It looks good now.

@jibran asked me to review this patch for wording/clarity/proofreading. I think it is looking great! It is very clear and it seems like a great addition to the documentation.

Here are 4 very small suggestions:

a)

+ * saves the source IDs, destination IDs and the row hash into a map table. The

The Drupal project uses serial commas: like "apples, oranges, and bananas" (comma before "and"). So there needs to be a comma after "IDs" in this line.

b) next line down:

+ * saves the source IDs, destination IDs and the row hash into a map table. The
+ * source IDs and the hash allow tracking changes for continuous migrations.

For that sentence, how about "facilitate" instead of "allow"?

c)

+ * In this example, the row property 'changed' is the high_water_property. If
+ * the value of 'changed' is greater than the current highwater mark the row
+ * is processed and the value of the highwater mark is updated to the value
+ * 'changed'.

Suggest putting "of" before that last word "changed".

d)

+ * When developing a migration, it is quite typical that the first version does
+ * not provide correct results for all scenarios. Rollbacks allow you to undo a
+ * migration and then execute it again after adjusting it.

Maybe instead of the word "scenarios" here, substitute something like "migrated data"?

@jhodgdon, thanks for taking the time to review this.

It is very clear and it seems like a great addition to the documentation.

Credit goes to masipila, who made great documentation contributions to Migrate.

#18 a, b, c, and d) Fixed as suggested. For d) I wondered if it should be 'source data' instead of 'migrated data'. In the end I decided that either get the meaning across so used the suggestion by the more experienced writer.

Thanks! I will not mark it RTBC since I am not sure about the technical correctness, but to me it looks great.

Thank you @jhodgdon for the review. Yeah, the patch is ready IMO.

@jibran and @jhodgdon, thanks for the quick reviews. Feels good to get a 3 years old issue to RTBC in two days.

Random test failure, on Media_library.Drupal\Tests\media_library\FunctionalJavascript\WidgetUploadTest #3066447: [random test failure] Random failures building media library form after uploading image (WidgetUploadTest)

Setting back to RTBC

Great job here! I really like documentation improvements, especially for migrate API.

I checked the proposed changes via Grammarly and it suggested replacing "When a term is being migrated, it is possible that its parent term has not yet been migrated." with something more simple. Can it be something like "When a term is being migrated, its parent term can be not migrated yet."?

That suggestion changes the meaning of the sentence and it wouldn't make any sense at all with the remaining text, so I think we need to leave it as it is.

Good improvements.

Committed/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!