-
Notifications
You must be signed in to change notification settings - Fork 19
Default package installation to @stable instead of default branch #219
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
/gemini review |
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.
Code Review
This pull request changes the default package installation behavior to prefer @stable versions over the default development branch. The implementation correctly introduces this logic and includes a fallback mechanism for repositories without stable releases. However, a critical issue is that the feature tests in features/package-install.feature have not been updated to reflect these significant behavioral changes. I've also identified two areas with code duplication that could be refactored to improve code maintainability. My review includes specific suggestions for these improvements.
| $extracted_package_name = $matches[1]; | ||
| if ( '@stable' === $version ) { | ||
| $tag = $this->get_github_latest_release_tag( $extracted_package_name, $insecure ); | ||
| $version = $this->guess_version_constraint_from_tag( $tag ); | ||
| } | ||
| $package_name = $this->check_git_package_name( $extracted_package_name, $package_name, $version, $insecure ); |
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.
There's some code duplication here. The logic to resolve the @stable version is repeated here and again on lines 312-315 for shortened identifiers.
To improve maintainability and follow the DRY (Don't Repeat Yourself) principle, you could extract this logic into a private helper method. For example, a method like resolve_stable_version_constraint(\$package_name, \$insecure) could encapsulate this logic, making the code cleaner and easier to manage.
| if ( 20 !== (int) substr( $response->status_code, 0, 2 ) ) { | ||
| WP_CLI::warning( 'Could not guess stable version from GitHub repository, falling back to master branch' ); | ||
| return 'master'; | ||
| $default_branch = $this->get_github_default_branch( $package_name, $insecure ); | ||
| WP_CLI::warning( "Could not guess stable version from GitHub repository, falling back to {$default_branch} branch" ); | ||
| return "dev-{$default_branch}"; | ||
| } | ||
|
|
||
| $package_data = json_decode( $response->body ); | ||
| if ( JSON_ERROR_NONE !== json_last_error() ) { | ||
| WP_CLI::warning( 'Could not guess stable version from GitHub repository, falling back to master branch' ); | ||
| return 'master'; | ||
| $default_branch = $this->get_github_default_branch( $package_name, $insecure ); | ||
| WP_CLI::warning( "Could not guess stable version from GitHub repository, falling back to {$default_branch} branch" ); | ||
| return "dev-{$default_branch}"; | ||
| } |
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.
The fallback logic to get the default branch is duplicated in the case of an HTTP error and a JSON parsing error. You can refactor this to avoid repetition and make the code cleaner by combining the checks. This also provides an opportunity to make the check for a valid response object more robust.
$package_data = null;
if ( 20 === (int) substr( $response->status_code, 0, 2 ) ) {
$package_data = json_decode( $response->body );
}
if ( ! is_object( $package_data ) || JSON_ERROR_NONE !== json_last_error() ) {
$default_branch = $this->get_github_default_branch( $package_name, $insecure );
WP_CLI::warning( "Could not guess stable version from GitHub repository, falling back to {$default_branch} branch" );
return "dev-{$default_branch}";
}
wp package installcurrently defaults to the repository's default branch (dev-main/dev-master), causing failures when that branch is misconfigured but stable releases exist. Changes default behavior to install from@stable.Changes
Installation flow:
@stableinstead ofdev-{default_branch}Fallback behavior when no releases exist:
dev-{actual_branch}(e.g., dev-main, dev-master)"Could not guess stable version from GitHub repository, falling back to {branch} branch"Modified functions:
install(): Lines 227, 309 - default to@stableget_github_latest_release_tag(): Lines 1367-1388 - fallback logic usingget_github_default_branch()Examples
Users can still explicitly specify versions:
:@stable- explicit stable (now redundant but supported):dev-main- development branch:v1.0.0- specific versionWarning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
example.com/usr/bin/php /usr/bin/php /home/REDACTED/work/package-command/package-command/vendor/wp-cli/wp-cli/bin/../php/boot-fs.php package install REDACTED package path d/vendor/wp-cli/wp-cli/bin/wp install(dns block)github.com (HTTP Only)/usr/bin/ssh /usr/bin/ssh -o SendEnv=GIT_PROTOCOL git@github.com git-upload-pack 'non-existent-git-user-asdfasdf/non-existent-git-repo-asdfasdf.git' list(packet block)/usr/bin/ssh /usr/bin/ssh -o SendEnv=GIT_PROTOCOL git@github.com git-upload-pack 'wp-cli-test/private-repository.git' /home/REDACTED/work/package-command/package-command/vendor/wp-cli/wp-cli/bin/wp git ndor/bin/git --local user.name /usr/sbin/bash sh wp-c�� package install n/sh tup_errors=0' '-git(packet block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.