-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move Migrate Run commands in Drush core #4606
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick initial review. Thanks for your hard work on this.
d982056
to
c086cda
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a composer conflict with migrate_tools module <=5? Would prevent some chaos around duplicate commands being declared.
0dde87c
to
ce93a7e
Compare
Good point. I've added also |
7d1f29b
to
4e60e8d
Compare
a21f64e
to
d1abad7
Compare
Thanks @claudiu-cristea. Great work. |
Fixes #2926. Development started in #3402.
Status of porting changes from Migrate Tools since #3402
::testImportingWithLimitAndFeedback()
.--idlist
is needed for messages. Also,--idlist-delimiter
was not added because: (1) the name is misleading, actually it refers to the delimiter of columns from the same ID but the name could make you think that is the delimiter between IDs and (2) I don't think there's a real use case to use something different than colon ':'. But if there is a real need, we need a better name and understand that edge case.::testImportingWithUpdateAndIdlist()
.--delete
option instead of--sync
.Follow-ups