Skip to content

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

Merged
schlessera merged 15 commits intowp-cli:masterfrom
nylen:add/download-from-url
Oct 22, 2019
Merged

Support wp core download https://somesite/build.zip#135
schlessera merged 15 commits intowp-cli:masterfrom
nylen:add/download-from-url

Conversation

@nylen
Copy link
Contributor

@nylen 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 a team as a code owner October 15, 2019 04:26
@nylen
Copy link
Contributor Author

nylen commented Oct 15, 2019

#136 should fix the build.

Copy link
Member

@schlessera schlessera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@schlessera
Copy link
Member

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

@schlessera schlessera added the command:core-download Related to 'core download' command label Oct 15, 2019
@nylen nylen force-pushed the add/download-from-url branch from 2c78c12 to 269a7dd Compare October 15, 2019 14:07
@nylen nylen requested a review from schlessera October 15, 2019 14:23
@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
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.

nylen and others added 7 commits October 20, 2019 21:24
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
Copy link
Member

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!

@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
@nylen nylen deleted the add/download-from-url branch October 23, 2019 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

command:core-download Related to 'core download' command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support wp core download https://somesite/build.zip

2 participants