-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add core release
command
#109
Conversation
Abstract `wp core verify-checksums` to wp-cli/checksum-command
Run Travis tests against WP-CLI dev-master
Update tests for WP 4.7.4
Fix Travis CI caching for composer
Update tests for WP 4.7.5
For PHP 7.1 compat use get_wp_details() instead of "version.php" include.
Use {WP_VERSION-latest} vars to avoid needing to update tests on WP releases.
Clarify this command as a bundled command
Replace instances of `wp.dev` with `example.com`
Implement CS checking based on the `WP_CLI_CS` ruleset
Co-Authored-By: wojsmol <[email protected]>
Change `CoreUpgrader::download_package` signature
Enhancement: Add PHP 7.3 to Travis CI build matrix
Lock framework version to ^2.2
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.
@schlessera Can you please merge latest master
here, for CS changes?
I would suggest to update example output to include WordPress 5.2 release as well - with an actual change in required PHP version it would make the examples even more telling. |
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.
Only minor query, rest LGTM.
$response = Utils\http_request( 'GET', $url ); | ||
|
||
if ( strncmp( (string) $response->status_code, '2', 1 ) ) { | ||
WP_CLI::error( |
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.
Could we avoid usage of sprintf
?
$semver_regex = '/^v?(\d+)(?:\.(\d+))?(?:\.(\d+))?(?:-([0-9A-Z-.]+))?(?:\+([0-9A-Z-.]+)?)?$/i'; | ||
|
||
if ( ! preg_match( $semver_regex, $version, $matches ) ) { | ||
WP_CLI::error( |
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.
Same as above.
Also as @chesio mentioned, an example of |
Shouldn't this be called |
I think the "resource" we're dealing with here is "a release", and you can fetch a list of multiples of this resource. Just as we have For context, the main use case I have for this is to turn a generic "release to use for tests" into "the latest release matching this criteria to use for tests". So, if I state that I want to run tests with |
BTW, this is still unmerged because the endpoint I'm using doesn't provide everything I need and I'm not sure I'm using the correct endpoint in the first place (see #108 for more details). |
I'm not sure the same principle applies - all the other examples have multiple subcommands, and in that case it should be When I read |
It seems like there are 2 uses cases then: listing all releases (optionally matching some criteria), or finding the latest release (optionally matching some criteria). Tying back into the "resource" discussion, why not make this into a proper resource with appropriate subcommands for these cases? For example:
Using |
Proceeding with wp-cli/wp-cli#5594 for this repository. I've captured this PR to https://gist.github.com/danielbachhuber/c3f452c41ae22532df1123550fec5f84 in case this PR is auto-closed or broken in some way. |
d289b58
to
762c7ce
Compare
Adds retrieval of available releases from the wordpress.org API server.
Version constraints are interpreted in the following way right now:
4
=> anything that starts with the major version 4. This would mean>= 4.0.0 && < 5.0.0
.4.8
=> anything that starts with the major version 4 and minor version 8. This would mean>= 4.8.0 && < 4.9.0
Some notes:
--locale
flag, but removed it again for now, as the API server does not seem to respect it for all results, only for the very latest version.Fixes #108