Add 386 architecture mappings and GitHub Proxy support
I use main fork in my project,I will cancel the old pr and now reopen a new PR。
Summary by CodeRabbit
-
New Features
- Added support for downloading assets via GitHub Proxy
- Extended 386 architecture support with additional architecture aliases
-
Tests
- Enhanced test coverage for 386 architecture variants
✏️ Tip: You can customize this high-level summary in your review settings.
Walkthrough
The changes extend architecture mapping to support 386 aliases (i386, x86, x86_32), add corresponding test cases for 386 architecture handling, and implement GitHub Proxy asset download detection with direct HTTP GET requests triggered by multiple "https://" occurrences in AssetURL.
Changes
| Cohort / File(s) | Change Summary |
|---|---|
Architecture mapping enhancementsarch.go, arch_test.go |
Adds conditional logic to map "386" architecture to additional aliases (i386, x86, x86_32); extends TestAdditionalArch with two new test cases covering 386 with and without universal arch |
GitHub Proxy asset download handlinggithub_source.go |
Detects proxy usage by counting "https://" occurrences in AssetURL; when detected, performs direct HTTP GET request instead of standard GitHub API path; selects download URL based on AssetID matching; adds strings import |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
- github_source.go: Proxy detection via string occurrence counting and conditional URL selection logic requires careful verification of edge cases and error handling for HTTP non-OK responses
- arch.go: Verify that new 386 mappings are applied in correct order relative to universalArch handling
- arch_test.go: Confirm test expectations cover both universal and non-universal cases for 386
Poem
🐰 A rabbit hops through architectures new,
From x86 to 386, we brew,
GitHub proxies found in URLs so long,
Direct downloads whisper their song! 🎵
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately reflects the two main changes: adding 386 architecture mappings and GitHub Proxy support for asset downloads. |
✨ Finishing touches
- [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
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 help to get the list of available commands and usage tips.
Codecov Report
:x: Patch coverage is 14.28571% with 18 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 76.95%. Comparing base (d7d55b9) to head (496a584).
:warning: Report is 1 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| github_source.go | 5.26% | 17 Missing and 1 partial :warning: |
:x: Your patch check has failed because the patch coverage (14.29%) 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 #51 +/- ##
==========================================
- Coverage 77.07% 76.95% -0.12%
==========================================
Files 28 28
Lines 1435 1167 -268
==========================================
- Hits 1106 898 -208
+ Misses 279 218 -61
- Partials 50 51 +1
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 76.95% <14.29%> (-0.12%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Hi!
Thank you for this pull request! Sorry it took me some time to look at it.
I'm a little bit concerned that the check for the double https:// could break some other users workflow.
Have you tried to check if the Github API is returning the redirected url as a second argument?
On line 139 in the PR, the s.api.Repositories.DownloadReleaseAsset could return nil, redirectedURL, nil
If this is the case, we can download the asset manually once we know the Github API cannot.
From the GitHub documentation:
DownloadReleaseAsset returns an io.ReadCloser that reads the contents of the specified release asset. It is the caller's responsibility to close the ReadCloser. If a redirect is returned, the redirect URL will be returned as a string instead of the io.ReadCloser. Exactly one of rc and redirectURL will be zero.
Sorry I cannot try it myself as I don't know what kind of setup you have.
I didn't change your original code, It will be working as normal as it would be. My code will be work Only when the url has two "https" like https://githubporxy.com/https://raw.githubuserconten.com/...