Rename MigrationCreationTrait as it no longer creates migrations - it configures them

Yanking this for sprint mentoring to let someone else have a crack :)

It doesn't look like that the patch in #5 would run, but I think it would fail anyway (the file was not renamed). I also updated the comment and removed the todo. I am uploading an updated patch.

Looking at this at Drupal NOLA sprint, we are looking into getting it ready for RTBC.

We were reviewing this issue with @heddn, it looks simple enough to be RTBC.

I did a project-wise search making sure there's no any leftover references to the old name, no doc comment or anything.

Attempting what @alexpott suggested.

I think, since we are maintaining the old trait, the change is upward-compatible and can go into 8.1.x.

Not sure if the test fail is real or an instance of #2724871: Random failure in \Drupal\migrate_drupal_ui\Tests\d7\MigrateUpgrade7Test, let's retest.

seems to be passing except from one older one.

Ignore the failure, it is coming from #2724871: Random failure in \Drupal\migrate_drupal_ui\Tests\d7\MigrateUpgrade7Test.

Running the 8.1 test again to see how consistently this issue reproduces it. Looks like it passed the 8.2 tests.

I must really want that patch tested. Or I just goofed and upload the wrong thing for the interdiff.

Hey guys,

I went through the comments and noticed @alexpott suggested to mark it as deprecated to be removed on 8.3.x (#14), but the patch is saying it will be remove on 9.0.x.
I believe the confusion happened because @alexpott provided an example (#16) where 9.0.x was mentioned.

Should we change it to 8.3.x or is keeping 9.0.x also fine?

Just in case, I'm submitting a new patch changing it to 8.3.x as I've mentioned before.
If 9.0.x is fine, just ignore it.

I don't think we tend to designate a specific point release when something is going to go away. It is too hard to forecast the future. But that does bring up a good point. Typically we are more clear about stating "before".

New patch attached that copies more exactly the typical pattern.

So a) If you set up your git like this:

 [diff]
  renames = copies

...it's a lot easier to read the patch and see what's actually changed. https://www.drupal.org/documentation/git/configure

b) I was curious what changed in the new file from the old one, so I did this:

$ git checkout -- core/modules/migrate_drupal/src/MigrationCreationTrait.php
$ diff core/modules/migrate_drupal/src/MigrationCreationTrait.php core/modules/migrate_drupal/src/MigrationConfigurationTrait.php 
11,14c11
<  * Creates the appropriate migrations for a given source Drupal database.
<  *
<  * @todo https://www.drupal.org/node/2682585 The trait no longer creates
<  *   migrations.
---
>  * Configures the appropriate migrations for a given source Drupal database.
16c13
< trait MigrationCreationTrait {
---
> trait MigrationConfigurationTrait {

So no code changes, just the appropriate name and comment changes.

As far as deprecation, yah, we should be specific about when the code will be removed, so 9.0.0 is most appropriate. And we should say that it's been removed in the version target for the issue, so 8.1.x is fine as long as we're targeting 8.1.x.

I think it might be that we need to bump up to 8.2.x for an API change, but a maintainer can make that call, referencing #18.

Could be the random failure #2724871: Random failure in \Drupal\migrate_drupal_ui\Tests\d7\MigrateUpgrade7Test, so retesting.

Passed this time.

Passing now, so changing back to RTBC.

Sure it was the mentioned issue on random fails.
Now that issue is fixed and we shouldn't have this test failing again.