The Wayback Machine - https://web.archive.org/web/20191215125534/https://github.com/wp-cli/core-command/pull/135
Skip to content

/core-command

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
Merged

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

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.

Sign in to view
Copy link
Contributor Author

nylen commented Oct 15, 2019

#136 should fix the build.
@schlessera
schlessera requested changes Oct 15, 2019
View changes
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.

Sign in to view
Copy link
Member

schlessera commented Oct 15, 2019

#136 was merged, so you can pull in the latest master.
nylen and others added 4 commits Oct 15, 2019
@nylen
Support `wp core download --version=https://somesite/build.zip`
c6b28e8 
Closes #131.
@nylen
A long time ago in a galaxy far, far away...
1744d09
@nylen @schlessera
Add positional argument
3005395 
Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>
@nylen
Use a positional argument instead of `--version=` for URLs
Loading status checks…
269a7dd
@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
Show error when specifying both URL and version
Loading status checks…
3740dc5
@nylen
Style fix
Loading status checks…
4de0a8d
@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.

Sign in to view
Copy link
Contributor Author

nylen commented Oct 15, 2019 •
edited

@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
Empty commit to trigger rebuild
Loading status checks…
d137e2d
@schlessera
schlessera requested changes Oct 16, 2019
View changes
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
@nylen @schlessera
Use `list` to fetch positional arguments
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
4b330bf 
Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>
@nylen @schlessera
Clarify conditional
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
9ed326f 
Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>
@nylen @schlessera
Remove redundant comment
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
eeba24b 
Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>
@nylen @schlessera
Remove duplicate assignment (1/2)
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
f8d50a7 
Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>
@nylen @schlessera
Remove duplicate assignment (2/2)
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
8da0ea4 
Co-Authored-By: Alain Schlesser <alain.schlesser@gmail.com>
@nylen
Check for HTTP codes between 201 and 299
Loading status checks…
2ad8092
@nylen
Style fix
Loading status checks…
9bee85d
@schlessera

This comment has been minimized.

Sign in to view
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!
@schlessera
Replace list() with array_shift() for fetching optional positional ar…
Verified
This commit was signed with a verified signature.
schlessera Alain Schlesser
GPG key ID: 14D455078E18D81E Learn about signing commits
Loading status checks…
1fa06e0 
…gument
@schlessera schlessera added this to the 2.0.7 milestone Oct 22, 2019
@schlessera
schlessera approved these changes Oct 22, 2019
View changes
@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
Reviewers

@schlessera schlessera

Assignees
No one assigned
Labels
command:core-download
Milestone
  2.0.7
2 participants
@nylen @schlessera
You can’t perform that action at this time.