The migrate module is still considered an experimental module. As a result, its code and tests have been reviewed less than other more mature code. Attach are changes noted during a review of the files in core/modules/migrate/src/Plugin/builder/*.php and core/modules/migrate/src/Plugin/destination/*.php. These changes include adding docblocks, missing @param/@var/@return descriptions and a few coding style changes.

CommentFileSizeAuthor
#5 2624888-5-migrate-module-src-plugin-d8.patch19.9 KBlars toomre
#2 2624888-2-migrate-module-src-plugin-d8.patch21.94 KBlars toomre

Comments

Lars Toomre created an issue. See original summary.

lars toomre’s picture

lars toomre commented
Status: Active » Needs review
Related issues: +#2624660: Some fixes to migrate/src/Plugin/*.php files, +#2624640: Fixes to docs and code style for migrate/src/*.php files, +#2621486: Fixes to migrate/tests/src/Unit/*.php files, +#2609590: Correct incorrect use of 'id' string in migration system
StatusFileSize
new 2624888-2-migrate-module-src-plugin-d8.patch21.94 KB

Here is a patch that covers fourteen files in the migrate/src/Plugin/build and Plugin/destination directories. This issue is related to #2624660: Some fixes to migrate/src/Plugin/*.php files, #2624640: Fixes to docs and code style for migrate/src/*.php files, #2621486: Fixes to migrate/tests/src/Unit/*.php files and #2609590: Correct incorrect use of 'id' string in migration system. In combination they fix up most of the docblock issues in the migrate module and have been created so that they are manageable in size to review and commit.

benjy’s picture

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

Thanks.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2624888-2-migrate-module-src-plugin-d8.patch, failed testing.

lars toomre’s picture

lars toomre commented
Status: Needs work » Needs review
StatusFileSize
new 2624888-5-migrate-module-src-plugin-d8.patch19.9 KB

Patch in #2 no longer applied. Here is a re-roll.

quietone’s picture

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

Reroll looks good, thank you, changes are due to #2609590: Correct incorrect use of 'id' string in migration system being fixed.

So, back to RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 6fc83e4 and pushed to 8.0.x and 8.1.x. Thanks!

  • alexpott committed 5636af4 on 8.1.x 
    Issue #2624888 by Lars Toomre: More fixes to migrate/src/Plugin/*.php...

Status: Fixed » Closed (fixed)

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