Problem/Motivation

Following the example of #2937989: Support multiple node types in node source plugins we'd like to add documentation and examples to migration source plugins, so they're easier to work with.

This issue will cover plugins for taxonomy terms:

  • core/modules/taxonomy/src/Plugin/migrate/source/d6/Term.php
  • core/modules/taxonomy/src/Plugin/migrate/source/d6/TermLocalizedTranslation.php
  • core/modules/taxonomy/src/Plugin/migrate/source/d7/Term.php
  • core/modules/taxonomy/src/Plugin/migrate/source/d7/TermEntityTranslation.php
  • core/modules/taxonomy/src/Plugin/migrate/source/d7/TermLocalizedTranslation.php
  • core/modules/taxonomy/src/Plugin/migrate/source/d7/TermTranslation.php

Remaining tasks

  1. Review <- we are here
  2. Address the feedback
  3. Commit
CommentFileSizeAuthor
#6 ClejVJj - Imgur.png74.47 KBilgnerfagundes

Issue fork drupal-3189587

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Matroskeen created an issue. See original summary.

Matroskeen opened merge request !180

Matroskeen’s picture

Matroskeen
Primary language Ukrainian
Location 🇺🇦 Ukraine, Lutsk
CreditAttribution: Matroskeen at DevBranch, Evolving Web, Drupal Ukraine Community commented
Status: Active » Needs review
quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs review » Reviewed & tested by the community

I reviewed the MR and paused on the use of the hyphen in the comment. But it does seem fine to me, even though I am not an English grammar expert.

this looks like another nice doc addition.

Thx!

Matroskeen’s picture

Matroskeen
Primary language Ukrainian
Location 🇺🇦 Ukraine, Lutsk
CreditAttribution: Matroskeen at DevBranch, Evolving Web, Drupal Ukraine Community commented

Re-based merge request. Still applies cleanly 😇

ilgnerfagundes’s picture

ilgnerfagundes CreditAttribution: ilgnerfagundes at CI&T commented
FileSize
ClejVJj - Imgur.png74.47 KB

This is fine with me RTBC + 1

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented

@ilgnerfagundes, thanks for the comment. Stating what you did to review the patch would be more helpful than a screenshot. For this issue it would be good to know that you read all the new comments and that they made sense to you and there was not duplication or errors in the links in the @see.

I applied the patch and read the comments and this is a very good improvement. I tried to view just the changes using the Compare with previous version' link but it would not load due to changes to 1000+ files! That is why I applied the patch locally and reread the whole thing.

Yes, RTBC

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: +Documentation

Committed and pushed 0921d71ec0 to 9.2.x and 3cbce90f1d to 9.1.x. Thanks!

@ilgnerfagundes thank you for looking into this issue.

Posting screenshots of your codebase or CLI does not advance the issue, since the automated testing infrastructure tells us whether the patch applies correctly.

So, I've removed the issue credit for that screenshot. In the future, you can get credit for issues by reading the issue to understand its purpose, and posting your review or testing of that purpose. Thank you!

  • alexpott committed 0921d71 on 9.2.x 
    Issue #3189587 by Matroskeen: Add documentation for taxonomy terms...

  • alexpott committed 3cbce90 on 9.1.x 
    Issue #3189587 by Matroskeen: Add documentation for taxonomy terms...

Matroskeen closed merge request !180

ilgnerfagundes’s picture

ilgnerfagundes CreditAttribution: ilgnerfagundes at CI&T commented

but alex i just put the screenshot to prove that i downloaded the patch, i ran it and did the tests, people above didn't even put any print and still proved it worked and i didn't?

Status: Fixed » Closed (fixed)

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