lucatume / wpcli-wpbrowser-tests

7 stars 1 forks source link

What did you think about the process of writing a command? #1

Open danielbachhuber opened 7 years ago

danielbachhuber commented 7 years ago

What went well, and what could be improved? Any documentation changes you'd suggest?

lucatume commented 7 years ago

Starting from never having used Behat to run tests and never having developed a custom wp-cli package before I'd say it went smooth; the code is easy to read and work with and the scaffolding really put me in the pilot seat in seconds.

The only documentation change I'd suggest is in the command cookbook example composer.json file:

I cannot thank all of you enough for the work you've done and do maintaining wp-cli: amazing work.

danielbachhuber commented 7 years ago

Starting from never having used Behat to run tests and never having developed a custom wp-cli package before I'd say it went smooth; the code is easy to read and work with and the scaffolding really put me in the pilot seat in seconds.

Glad to hear 👍

the required PHP version is 5.4 which is perfectly fine but clashes with the default one set to 5.3; it's not a drama but the installation guide shows 5.3.29 as minimum requirement and I think it would make sense to keep that in the example.

Good suggestion, updated https://github.com/wp-cli/wp-cli.github.com/pull/248

the require section does not define a wp-cli requirement and that will lead to failures while installing the package; I've removed the line, tried the install and ran into an error; restored the line (see here) and it all went smooth.

Huh. I don't think the wp-cli requirement is actually necessary. In fact, WP-CLI doesn't currently handle this correctly, see https://github.com/wp-cli/wp-cli/issues/2675

Could you share more details about the error you ran into?

I cannot thank all of you enough for the work you've done and do maintaining wp-cli: amazing work.

You're welcome!

lucatume commented 7 years ago

Huh. I don't think the wp-cli requirement is actually necessary. In fact, WP-CLI doesn't currently handle this correctly, see wp-cli/wp-cli#2675

And it's not, having set it in my local version wp-cli own composer.lock file was not updated and stuck to the wrong version, totally my bad.