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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add `shuffle-salts` command #62

Merged
merged 12 commits into from Jul 26, 2018

Conversation

2 participants
@CodeProKid
Contributor

CodeProKid commented Jul 20, 2018

First pass at the shuffle-salts command here. This uses code fairly similar to what core uses here -> https://github.com/WordPress/wordpress-develop/blob/master/src/wp-admin/setup-config.php#L317-L346

I'm also relying on the internal unique_key method here which was added in #25

Working on adding tests now, just wanted to get the first pass up 馃憤

@CodeProKid

This comment has been minimized.

Show comment
Hide comment
@CodeProKid

CodeProKid Jul 20, 2018

Contributor

Woops, forgot that the issue for this was in a separate repo -> wp-cli/ideas#50

Contributor

CodeProKid commented Jul 20, 2018

Woops, forgot that the issue for this was in a separate repo -> wp-cli/ideas#50

@CodeProKid

This comment has been minimized.

Show comment
Hide comment
@CodeProKid

CodeProKid Jul 20, 2018

Contributor

Tests are up here now. Not sure if this needs more tests or not? Seems to cover the bases pretty good though. Will wait to make sure the build passes 馃憤

Contributor

CodeProKid commented Jul 20, 2018

Tests are up here now. Not sure if this needs more tests or not? Seems to cover the bases pretty good though. Will wait to make sure the build passes 馃憤

Show outdated Hide outdated src/Config_Command.php
Show outdated Hide outdated src/Config_Command.php
Show outdated Hide outdated src/Config_Command.php
@CodeProKid

This comment has been minimized.

Show comment
Hide comment
@CodeProKid

CodeProKid Jul 20, 2018

Contributor

@schlessera I made the changes you requested here. Looks like tests are still failing, but it seems to be due to timeouts on travis.

Contributor

CodeProKid commented Jul 20, 2018

@schlessera I made the changes you requested here. Looks like tests are still failing, but it seems to be due to timeouts on travis.

@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Jul 20, 2018

Member

@CodeProKid Seems like there is still a problem with this code with PHP 5.6: https://travis-ci.org/wp-cli/config-command/jobs/406183041#L648-L649
The problem might or might not be caused by the upstream wp-config-transformer library.

Member

schlessera commented Jul 20, 2018

@CodeProKid Seems like there is still a problem with this code with PHP 5.6: https://travis-ci.org/wp-cli/config-command/jobs/406183041#L648-L649
The problem might or might not be caused by the upstream wp-config-transformer library.

@CodeProKid

This comment has been minimized.

Show comment
Hide comment
@CodeProKid

CodeProKid Jul 24, 2018

Contributor

@schlessera I fixed the issue causing the tests to break, and added some documentation about the command in the readme.

Contributor

CodeProKid commented Jul 24, 2018

@schlessera I fixed the issue causing the tests to break, and added some documentation about the command in the readme.

Show outdated Hide outdated src/Config_Command.php
Show outdated Hide outdated README.md
@CodeProKid

This comment has been minimized.

Show comment
Hide comment
@CodeProKid

CodeProKid Jul 26, 2018

Contributor

@schlessera Good catch on the debug code 馃う鈥嶁檪锔 also, didn't realize that about the readme, pretty cool. I ran the command to regenerate it in the latest commit.

Contributor

CodeProKid commented Jul 26, 2018

@schlessera Good catch on the debug code 馃う鈥嶁檪锔 also, didn't realize that about the readme, pretty cool. I ran the command to regenerate it in the latest commit.

@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Jul 26, 2018

Member

@CodeProKid I forgot to mention that there's an extra step needed for the README.md regeneration, as you've added a new command (as you can see in the commit, it did not actually add the documentation).

You need to first add that new command to the 'extra.commands' section of the composer.json file: https://github.com/wp-cli/config-command/blob/v1.2.0/composer.json#L42-L52

After you've added the new command there, regenerate the README.md again and it will properly pick up the docblock of that new command.

Member

schlessera commented Jul 26, 2018

@CodeProKid I forgot to mention that there's an extra step needed for the README.md regeneration, as you've added a new command (as you can see in the commit, it did not actually add the documentation).

You need to first add that new command to the 'extra.commands' section of the composer.json file: https://github.com/wp-cli/config-command/blob/v1.2.0/composer.json#L42-L52

After you've added the new command there, regenerate the README.md again and it will properly pick up the docblock of that new command.

@CodeProKid

This comment has been minimized.

Show comment
Hide comment
@CodeProKid

CodeProKid Jul 26, 2018

Contributor

@schlessera ahhh, I see. Never knew about the auto doc generation for commands. Very cool! Should be good now 馃憤

Contributor

CodeProKid commented Jul 26, 2018

@schlessera ahhh, I see. Never knew about the auto doc generation for commands. Very cool! Should be good now 馃憤

@schlessera schlessera added this to the 1.2.1 milestone Jul 26, 2018

@schlessera schlessera merged commit aae296c into wp-cli:master Jul 26, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@schlessera schlessera changed the title from Feature/50 adding shuffle-salts command to Add `shuffle-salts` command Jul 26, 2018

@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Jul 26, 2018

Member

Thanks for the pull request, @CodeProKid !

Member

schlessera commented Jul 26, 2018

Thanks for the pull request, @CodeProKid !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment