Supports direct download assets through GitHub Proxy & Add support for 386 architecture#49
Supports direct download assets through GitHub Proxy & Add support for 386 architecture#49sinspired wants to merge 3 commits intocreativeprojects:mainfrom
Conversation
WalkthroughAdds "386" architecture aliases ("i386", "x86", "x86_32") to arch expansion and related tests. Adds a bypass in GitHub asset download to directly GET asset URLs that contain multiple "https://" prefixes; otherwise retains the GitHub API path. Updates module/import paths and documentation references to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant GS as GitHubSource.DownloadReleaseAsset
participant GH as GitHub Releases API
participant HTTP as Direct HTTP Server
Caller->>GS: DownloadReleaseAsset(params)
alt AssetURL contains multiple "https://"
note right of GS #EEE8D5: Bypass GitHub API (new path)
GS->>GS: Pick direct download URL (AssetID or ValidationAssetID)
alt URL found
GS->>HTTP: HTTP GET download URL
HTTP-->>GS: Response (status/body)
alt Status 200 OK
note right of GS #D6EAF8: Return response body (caller closes)
GS-->>Caller: io.ReadCloser (body)
else Non-OK status
GS-->>Caller: error (status)
end
else No suitable URL
GS-->>Caller: ErrAssetNotFound
end
else Normal path
note right of GS #F3E6FF: Use GitHub Releases API (existing path)
GS->>GH: Request asset download via API
GH-->>GS: Asset stream / redirect handled
GS-->>Caller: io.ReadCloser (body)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (16)
✅ Files skipped from review due to trivial changes (4)
🧰 Additional context used🧬 Code graph analysis (1)update_test.go (1)
🪛 markdownlint-cli2 (0.18.1)cmd/get-release/README.md6-6: Dollar signs used before commands without showing output (MD014, commands-show-output) 🔇 Additional comments (12)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
github_source.go (2)
114-122: Minor formatting: Add spaces after colons in error messages.For consistency with error formatting conventions, add spaces after colons.
req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadUrl, http.NoBody) if err != nil { - return nil, fmt.Errorf("failed to create download request:%w", err) + return nil, fmt.Errorf("failed to create download request: %w", err) } resp, err := http.DefaultClient.Do(req) if err != nil { - return nil, fmt.Errorf("download failed:%w", err) + return nil, fmt.Errorf("download failed: %w", err) }
97-102: Fix comment threshold mismatch and improve proxy detection heuristic.
- Update the comment to match the code’s
> 1check:- // If the AssetURL contains more than 2 "https://", it means it's using a GitHub Proxy service. + // If the AssetURL contains more than one "https://", it means it's using a GitHub Proxy service.- Replace the substring count with a stricter URL check (e.g. parse and validate the host or path) to avoid false positives.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
arch.go(1 hunks)arch_test.go(1 hunks)github_source.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
github_source.go (1)
errors.go (1)
ErrAssetNotFound(13-13)
🔇 Additional comments (2)
arch_test.go (1)
26-27: LGTM!The test cases correctly validate the new 386 architecture support with proper aliases ("i386", "x86", "x86_32") and universal architecture handling.
arch.go (1)
30-32: LGTM!The 386 architecture handling follows the established pattern and includes appropriate aliases commonly used for 32-bit x86 binaries.
|
I need it in my project. |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (22.58%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #49 +/- ##
==========================================
- Coverage 77.07% 75.92% -1.15%
==========================================
Files 28 28
Lines 1435 1466 +31
==========================================
+ Hits 1106 1113 +7
- Misses 279 302 +23
- Partials 50 51 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| "log" | ||
| "os" | ||
|
|
||
| "github.com/creativeprojects/go-selfupdate" |
There was a problem hiding this comment.
Can you please fix all the paths back to the original ones? as it stands it's all broken when using the paths from your fork.
Thanks 👍🏻
There was a problem hiding this comment.
I will close this PR and open a new PR to fix the paths。
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation