Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add support GitHub releases/latest on installation #386

Conversation

BogdanUngureanu
Copy link
Contributor

@BogdanUngureanu BogdanUngureanu commented Nov 17, 2023

Related to #91

This PR adds support for installing plugins and themes from GitHub using the release/latest URLs.

In order to get the latest release assets, I'm proposing to use the GitHub public API to extract the latest version of the plugin/theme from GH releases. If there's no asset defined, we will fall back to the source code zip file.

While the implementation works right now, I haven't added support for GitHub's API request limits. Currently, after 60 requests per hour, the user is required to use an access token in order to make an API request. However, this PR doesn't expose a way to pass that token to the request.

UPDATE:
I've introduced a new environment variable called GITHUB_TOKEN.

@BogdanUngureanu BogdanUngureanu requested a review from a team as a code owner November 17, 2023 09:16
@swissspidy
Copy link
Member

  • Pass a new optional GitHub access token as a flag on the command.

I'd use an environment variable instead, e.g. getenv( 'GITHUB_TOKEN' )

@BogdanUngureanu
Copy link
Contributor Author

Hey @swissspidy, that's a great idea! I've updated the PR to contain the environment variable you proposed. Thanks!

@BogdanUngureanu BogdanUngureanu changed the title WIP: Add support GitHub releases/latest on installation Add support GitHub releases/latest on installation Nov 18, 2023
Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

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

Looks like a great start, @BogdanUngureanu ! I have a few changes to suggest.

*
* @return string|null
*/
public function get_github_repo_from_url( $url ) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public function get_github_repo_from_url( $url ) {
public function get_github_repo_from_releases_url( $url ) {

Should we include 'releases' in this method name to make it more specific?


namespace WP_CLI;

final class ParseGithubUrlInput {
Copy link
Member

Choose a reason for hiding this comment

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

Can we move the methods on this class to the existing CommandWithUpgrade class? I don't think we want to introduce it as a class others can use, etc.

"""
Plugin installed successfully.
Success: Installed 1 of 1 plugins.
"""
Copy link
Member

Choose a reason for hiding this comment

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

Can you include a test case for the non-happy path too?

@swissspidy
Copy link
Member

Closing in favor of #421

@swissspidy swissspidy closed this Apr 26, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants