Skip to content
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

Migrate term from a taxonomy to another one #234

Merged
merged 13 commits into from Feb 24, 2019

Conversation

@Mte90
Copy link
Contributor

commented Feb 13, 2019

The new command is avalaible!

I need only to do the unit tests but I will work on them but before to do that I want a feedback about the warning in the code.

Example output:

Post 218854 assigned to new term!
Post 218860 assigned to new term!
Post 218870 assigned to new term!
Post 218879 assigned to new term!
Post 218892 assigned to new term!
Post 218893 assigned to new term!
Post 218903 assigned to new term!
Post 218913 assigned to new term!
Post 218922 assigned to new term!
Post 218957 assigned to new term!
Post 218974 assigned to new term!
Post 218995 assigned to new term!
Post 219050 assigned to new term!
Post 219060 assigned to new term!
Term migrated!
Old Term removed!
Success: Migration of `9190` term for 5494 posts
@desrosj

This comment has been minimized.

Copy link

commented Feb 15, 2019

@Mte90 Are you able to write some tests for this?

@Mte90

This comment has been minimized.

Copy link
Contributor Author

commented Feb 15, 2019

Yes I can do it but I am not sure about the warning (specifically about the text messages) printed in the console.

@schlessera

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

Let me do a quick review of the text messages then so you now what final strings to test against...

src/Term_Command.php Outdated Show resolved Hide resolved
src/Term_Command.php Outdated Show resolved Hide resolved
src/Term_Command.php Outdated Show resolved Hide resolved
src/Term_Command.php Outdated Show resolved Hide resolved
src/Term_Command.php Outdated Show resolved Hide resolved
src/Term_Command.php Outdated Show resolved Hide resolved
src/Term_Command.php Outdated Show resolved Hide resolved
src/Term_Command.php Show resolved Hide resolved
src/Term_Command.php Show resolved Hide resolved
src/Term_Command.php Show resolved Hide resolved

schlessera and others added some commits Feb 19, 2019

Apply suggestions from code review
Co-Authored-By: Mte90 <mte90net@gmail.com>
@Mte90

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2019

I will work on the other changes and tests in the next days.

Mte90 added some commits Feb 22, 2019

@schlessera
Copy link
Member

left a comment

A few small string changes, and then we're good to merge.

features/term-migrate.feature Outdated Show resolved Hide resolved
features/term-migrate.feature Outdated Show resolved Hide resolved
features/term-migrate.feature Outdated Show resolved Hide resolved
features/term-migrate.feature Outdated Show resolved Hide resolved
features/term-migrate.feature Outdated Show resolved Hide resolved
src/Term_Command.php Outdated Show resolved Hide resolved
features/term-migrate.feature Outdated Show resolved Hide resolved
features/term-migrate.feature Outdated Show resolved Hide resolved
features/term-migrate.feature Outdated Show resolved Hide resolved
src/Term_Command.php Outdated Show resolved Hide resolved
Apply suggestions from code review
apply second review

Co-Authored-By: Mte90 <mte90net@gmail.com>

@schlessera schlessera added this to the 2.0.3 milestone Feb 24, 2019

@schlessera schlessera merged commit 6132db5 into wp-cli:master Feb 24, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@schlessera

This comment has been minimized.

Copy link
Member

commented Feb 24, 2019

Thanks for the PR, @Mte90!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.