Document public function MigrateSourceInterface::getIds return value better

The schema arrays here are the same ones used in hook_schema() (see https://www.drupal.org/node/159605) - perhaps an @see would be helpful.

The 'alias' is a migrate extension, that should be documented here.

Regarding the 'alias' key. It looks like this is currently only used in \Drupal\migrate\Plugin\migrate\source\SqlBase::initializeIterator, where it's used to determine which field from the source query is the one in question.

Example:

$field_name = $field_schema['alias'] . '.' . $this->query->escapeField($field_name);

So maybe adding something like; "In addition, the migrate system uses an 'alias' key to distinguish between ambiguous field names in some situations. For example when a SQL source query joins two tables with the same field. Providing an optional alias results in using {alias}.{field_name} being used as the ID field."

Or something like that. While I understand what is going on here I had a hard time figuring out how to explain this in a generic way.

I find it a little bit weird that this is documented on the interface, but not necessarily implemented every time. For example, how does the 'alias' key apply if you're using a JSON source? How would you even know what to set it to in that case?

Make this a child of the migration documentation Meta issue.

The documentation for getIds was greatly improved by @claudiu.cristea in #2783715: Entity destination ID schema should match entity ID definition. I've added the suggested text in #3. Also moved the first example to it's own paragraph and added an 'Examples:' heading. At least for me, it is easier to both skim the examples and to find the related text.

Self-assigning for review.

1. +++ b/core/modules/migrate/src/Plugin/MigrateSourceInterface.php
   @@ -82,7 +85,10 @@ public function __toString();
   +   *   distinguish between ambiguous field names, for example, when a SQL source
   

   Let's say "column names", since "field" is a loaded term in Drupal. Also, can we have a hyphen after "field names", not a comma?

2. +++ b/core/modules/migrate/src/Plugin/MigrateSourceInterface.php
   @@ -82,7 +85,10 @@ public function __toString();
   +   *   query joins two tables with the same field. Providing an optional alias
   

   "Same field" is weird phrasing. How about "...joins two tables which have some of the same column names"? Let us also strike the word "optional" from this line; alternately, we can remove the final sentence altogether because it is an implementation detail specific to SqlBase and therefore arguably does not belong on the interface.

Thanks! Small thing:

+++ b/core/modules/migrate/src/Plugin/MigrateSourceInterface.php
@@ -86,9 +86,8 @@
+   *   distinguish between ambiguous column names- for example, when a SQL source

There needs be a space after "names", before the hyphen, and "source" exceeds the 80-character limit for the line.

Other than that, setting back to NW until #9.2 is addressed.

@phenaproxima Fixed 10.1 and regarding 9.2 as per your suggestion i removed last sentence of the paragraph.Keeping it to Needs work for now.

Unless I'm missing something here, I think all feedback has been addressed. I did a review of the docs changes and the description of getIds() seems to make sense to me. The two odd things that previously were a bit mysterious are now explained: alias and field storage settings.

+++ b/core/modules/migrate/src/Plugin/MigrateSourceInterface.php
@@ -82,7 +85,9 @@ public function __toString();
+   *   along the ID definition, is 'alias' used by SqlBase source plugin to

by *the* SqlBase source plugin?

Actually, should the SqlBase-specific 'alias' really be documented in MigrateSourceInterface?

re: #15, I think the idea was to provide an example of how custom values could be stored in the ids. Should this be spelled out more clearly?

Please provide an interdiff. I cannot tell what the changes are without one. See https://www.drupal.org/documentation/git/interdiff

+++ b/core/modules/migrate/src/Plugin/MigrateSourceInterface.php
@@ -82,7 +85,9 @@ public function __toString();
    *   definition, can be passed in definitions. The most common setting, passed
...
+   *   along the ID definition, is 'alias' used by the SqlBase source plugin to

I think we can do better English here. The phrasing doesn't flow.

And per Mike's comment, let's add an @see to the SqlBase plugin, and shorten the description here a little more to give the idea that there is use of passing along additional key/values. But to *really* read up on it, go see the examples in their real location.

I'll wrap this quite old api documentation issue later today.

One more old API documentation issue to be finalized, here we go.

Re: #20

1. This patch addresses the 'flow' of the phrasing. I also improved the flow of the description of the first code example.

2. There was already @see \Drupal\migrate\Plugin\migrate\source\SqlBase

3. @heddn, did you mean by your last comment in #20 that we should remove the last code example? I did not remove it, I think it is a good example. If the developers are reading the API documentation of the SqlBase source plugin class they can then follow a link to MigrateSourceInterface::getIds() from that context.

4. I also separated the code examples from each other by one newline. The coding standards say that we must not use a blank line between the text that explains the code sample and the code sample itself. The newlines I added are to separate examples from each other i.e. we are not talking about newlines between the code and its description.

Cheers,
Markus

Two supernits! I've outdone myself.

1. +++ b/core/modules/migrate/src/Plugin/MigrateSourceInterface.php
   @@ -80,9 +84,13 @@ public function __toString();
   +   *   passed along the ID definition is 'alias' used by the SqlBase source
   

   There should be a comma after 'alias'.

2. +++ b/core/modules/migrate/src/Plugin/MigrateSourceInterface.php
   @@ -80,9 +84,13 @@ public function __toString();
   +   *   example when a SQL source query joins two tables with the same column
   

   There should also be a comma after 'example'.

re #23 you've addressed my feedback quite well. There's one more nit I see not called out by Adam. Its pasted below. I think we need the word to connect the phrases more cleanly.

+++ b/core/modules/migrate/src/Plugin/MigrateSourceInterface.php
@@ -80,9 +84,13 @@ public function __toString();
+   *   definition can be added as shown below. The most common setting
+   *   passed along the ID definition is 'alias' used by the SqlBase source

The most common setting passed along to the ID definition...

Three nits from #24-25 addressed with two line changes, haha, beat you by one! :D

Issue summary updated to help committers.

Cheers,
Markus

All feedback addressed.

Thanks all, this is clearly an improvement even if just a small one :)