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

Support `wp core download https://somesite/build.zip` #135

Merged
merged 15 commits into from Oct 22, 2019

Conversation

@nylen
Copy link
Contributor

nylen commented Oct 15, 2019

Closes #131.

This change also makes a failure to retrieve the MD5 checksum into a warning rather than an error, and fixes a related minor bug along the way:

- WP_CLI::error( "Couldn't access md5 hash for release (HTTP code {$response->status_code})." );
+ WP_CLI::error( "Couldn't access md5 hash for release (HTTP code {$md5_response->status_code})." );

Previously this printed Couldn't access md5 hash for release (HTTP code 200) using the response code from the zip download.

It'll be slightly easier to review this diff ignoring whitespace changes: https://github.com/wp-cli/core-command/pull/135/files?w=1

@nylen nylen requested a review from wp-cli/committers as a code owner Oct 15, 2019
@nylen

This comment has been minimized.

Copy link
Contributor Author

nylen commented Oct 15, 2019

#136 should fix the build.

Copy link
Member

schlessera left a comment

The code generally looks good, but I'd like to have the mechanism for providing the URL changed to not be hidden under the misleading --version=<version> flag.

src/Core_Command.php Show resolved Hide resolved
@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Oct 15, 2019

#136 was merged, so you can pull in the latest master.

@nylen nylen force-pushed the nylen:add/download-from-url branch from 2c78c12 to 269a7dd Oct 15, 2019
nylen added 2 commits Oct 15, 2019
@nylen nylen requested a review from schlessera Oct 15, 2019
@nylen nylen changed the title Support `wp core download --version=https://somesite/build.zip` Support `wp core download https://somesite/build.zip` Oct 15, 2019
@nylen

This comment has been minimized.

Copy link
Contributor Author

nylen commented Oct 15, 2019

@schlessera thanks for the review, this is ready for another look.

The Travis build has passed, but the PR hasn't updated accordingly. Maybe clicking Rebuild on one of the jobs will clear that up? I pushed an empty commit to try again, which seems to have worked.

src/Core_Command.php Outdated Show resolved Hide resolved
src/Core_Command.php Outdated Show resolved Hide resolved
src/Core_Command.php Outdated Show resolved Hide resolved
src/Core_Command.php Outdated Show resolved Hide resolved
src/Core_Command.php Outdated Show resolved Hide resolved
src/Core_Command.php Outdated Show resolved Hide resolved
nylen and others added 7 commits Oct 20, 2019
Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>
Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>
Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>
Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>
Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>
@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Oct 22, 2019

Ah, just noticed that the list() does not even work if the positional is optional, so that suggestion was rather silly.

I'll take this over the finish line from here.

Thanks so much for the PR!

…gument
@schlessera schlessera added this to the 2.0.7 milestone Oct 22, 2019
@schlessera schlessera merged commit bb0e929 into wp-cli:master Oct 22, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@nylen nylen deleted the nylen:add/download-from-url branch Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.