Follow-up to #2694009: MigrationInterface doesn't include get()

Problem/Motivation

#2694009: MigrationInterface doesn't include get() deprecated get() in favor of specific getters. I've found one bit of configuration I'd like to get out of migration plugins that doesn't have a getter, though - migration_tags.

Proposed resolution

Add getMigrationTags().

Remaining tasks

Do it. Code it.

User interface changes

API changes

Adds public function getMigrationTags(). to MigrationInterface and Migration plugin.

Data model changes

CommentFileSizeAuthor
#6 add_getter_for-2741385-6.patch1.39 KBmikeryan
#4 interdiff.txt773 bytesmikeryan
#4 add_getter_for-2741385-4.patch1.38 KBmikeryan
#2 add_getter_for-2741385-2.patch1011 bytesmikeryan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mikeryan created an issue. See original summary.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Virtuoso Performance commented
Issue summary: View changes
Status: Active » Needs review
FileSize
add_getter_for-2741385-2.patch1011 bytes
benjy’s picture

benjy CreditAttribution: benjy at PreviousNext commented
Status: Needs review » Needs work 
+++ b/core/modules/migrate/src/Plugin/Migration.php
@@ -713,4 +713,11 @@ public function getDestinationIds() {
+    return isset($this->migration_tags) ? $this->migration_tags : [];

Can we not set this as a property on the class then just return $this->migration_tags?

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Virtuoso Performance commented
Status: Needs work » Needs review
FileSize
add_getter_for-2741385-4.patch1.38 KB
interdiff.txt773 bytes

Ayup.

Status: Needs review » Needs work

The last submitted patch, 4: add_getter_for-2741385-4.patch, failed testing.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Virtuoso Performance commented
Status: Needs work » Needs review
FileSize
add_getter_for-2741385-6.patch1.39 KB

For want of a pull, the patch was lost...

benjy’s picture

benjy CreditAttribution: benjy at PreviousNext commented
Status: Needs review » Reviewed & tested by the community

Looks good.

  • catch committed 2491015 on 8.2.x 
    Issue #2741385 by mikeryan, benjy: Add getter for migration_tags
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.2.x, thanks!

After committing I realised this needs a change notice, so wrote one, feel free to improve: https://www.drupal.org/node/2752067

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Virtuoso Performance commented

Tweaked the change notice, thanks!

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Virtuoso Performance commented

I've published the change notice.

Status: Fixed » Closed (fixed)

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