Conversation
|
Woops, forgot that the issue for this was in a separate repo -> wp-cli/ideas#50 |
|
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 👍 |
src/Config_Command.php
Outdated
|
|
||
| } | ||
|
|
||
| $constant_list = array( 'AUTH_KEY', 'SECURE_AUTH_KEY', 'LOGGED_IN_KEY', 'NONCE_KEY', 'AUTH_SALT', 'SECURE_AUTH_SALT', 'LOGGED_IN_SALT', 'NONCE_SALT' ); |
There was a problem hiding this comment.
Please make this multi-line, to make it more legible:
$constant_list = array(
'AUTH_KEY',
'SECURE_AUTH_KEY',
'LOGGED_IN_KEY',
'NONCE_KEY',
'AUTH_SALT',
'SECURE_AUTH_SALT',
'LOGGED_IN_SALT',
'NONCE_SALT'
);
src/Config_Command.php
Outdated
| public function shuffle_salts( $args, $assoc_args ) { | ||
|
|
||
| try { | ||
| for ( $i = 0; $i < 8; $i++ ) { |
There was a problem hiding this comment.
Please directly couple this to the number of known constants:
foreach ( $constant_list as $key ) {
$secret_keys[ $key ] = self::unique_key();
}
You'll have to pull the declaration of $constant_list higher up for this.
src/Config_Command.php
Outdated
| try { | ||
| $config_transformer = new WPConfigTransformer( $path ); | ||
| foreach ( $constant_list as $constant ) { | ||
| $config_transformer->update( 'constant', $constant, $secret_keys[ $key ] ); |
There was a problem hiding this comment.
Use $secret_keys[ $constant ] here instead. You can get rid of the numeric $key completely.
|
@schlessera I made the changes you requested here. Looks like tests are still failing, but it seems to be due to timeouts on travis. |
|
@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 |
|
@schlessera I fixed the issue causing the tests to break, and added some documentation about the command in the readme. |
src/Config_Command.php
Outdated
| foreach ( $constant_list as $key ) { | ||
| $secret_keys[ $key ] = trim( self::unique_key() ); | ||
| } | ||
| throw new Exception( 'TEST' ); |
There was a problem hiding this comment.
Looks like there's some debugging left-overs here.
| ~~~ | ||
| wp config shuffle-salts | ||
| ~~~ | ||
|
|
There was a problem hiding this comment.
Looks like you manually changed the README.md file. This should be generated out of the source code docblock instead. The automatic regeneration is not yet included here so yu need to do this manually for now.
You can use the following to regenerate the README.md file:
- Make sure the required package is installed (this one is not bundled)
wp package install wp-cli/scaffold-package-command - Force a regeneration the
README.mdfilewp scaffold package-readme . --force
|
@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. |
|
@CodeProKid I forgot to mention that there's an extra step needed for the You need to first add that new command to the After you've added the new command there, regenerate the |
|
@schlessera ahhh, I see. Never knew about the auto doc generation for commands. Very cool! Should be good now 👍 |
shuffle-salts command
|
Thanks for the pull request, @CodeProKid ! |
Feature/50 adding shuffle-salts command
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_keymethod here which was added in #25Working on adding tests now, just wanted to get the first pass up 👍