Problem/Motivation

In #3252386: Use PHP attributes instead of doctrine annotations we added support for attribute based plugin discovery.
As part of that issue we converted block and action plugins.

This issue is to convert \Drupal\migrate\Annotation\MigrateDestinationplugins to use Attributes.

Proposed resolution

Convert remaining MigrateDestination plugins to attributes

Remaining tasks

#3424509: Update MigratePluginManager to include both attribute and annotation class
Review
Commit

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 3421015-nr-bot.txt12.62 KBneeds-review-queue-bot

Issue fork drupal-3421015

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

larowlan created an issue. See original summary.

godotislate made their first commit to this issue’s fork.

godotislate opened merge request !6798

godotislate’s picture

godotislate
he/him
commented
Status: Active » Needs review

Note that change to `core/modules/migrate/src/Plugin/MigratePluginManager.php` is also needed by other migrate plugins.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3421015-nr-bot.txt12.62 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

godotislate’s picture

godotislate
he/him
commented
Status: Needs work » Needs review

Rebased.

mohit_aghera’s picture

mohit_aghera commented

Note: BC approach to handle annotations and attributes in MigratePluginManager class is being evaluated in https://www.drupal.org/project/drupal/issues/3424509

We should revisit and rebase this PR after this is resolved.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Searched for @MigrateDestination and all 31 instances in core have been replaced.

Checked the attribute for the deriver class and no @Annotation.

Believe this one is good.

godotislate’s picture

godotislate
he/him
commented
Status: Reviewed & tested by the community » Postponed

Blocked on #3424509: Update MigratePluginManager to include both attribute and annotation class.

catch’s picture

catch
he/him
Primary language English
commented
Status: Postponed » Needs review
quietone’s picture

quietone commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes
Status: Needs review » Postponed
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Postponed » Closed (duplicate)
godotislate’s picture

godotislate
he/him
commented
Status: Closed (duplicate) » Needs review

Re-opening because the balance of destination plugins outside a couple done in #3424509: Update MigratePluginManager to include both attribute and annotation class still need to be converted. MR here has been rebased and is ready for review.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Believe 2 were missed EntityFile and Book.php

godotislate’s picture

godotislate
he/him
commented
Status: Needs work » Needs review

Believe 2 were missed EntityFile and Book.php

Those two were already done in #3424509: Update MigratePluginManager to include both attribute and annotation class
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/file/...
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/book/...

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

You are correct, that's my mistake

Rebase my local and applying MR all instances of @MigrateDestination have been replaced.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 38e722aa6f to 11.x and 3f64e2bf2c to 10.3.x. Thanks!

  • alexpott committed 3f64e2bf on 10.3.x 
    Issue #3421015 by godotislate: Convert MigrateDestination plugin...

  • alexpott committed 38e722aa on 11.x 
    Issue #3421015 by godotislate: Convert MigrateDestination plugin...

quietone closed merge request !6798

Status: Fixed » Closed (fixed)

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