start refresh #19
Merged
start refresh #19
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This pull request focuses on updating dependencies and adding ARM64 support for Linux platforms. The changes primarily involve upgrading Go version to 1.23, updating GitHub API client library from v44 to v73, replacing the archiver library with a new archives library, and enhancing CI workflows.
- Updates Go version from 1.18 to 1.23.0 with comprehensive dependency upgrades
- Adds ARM64 architecture support for Linux asset detection
- Replaces archiver/v4 library with mholt/archives for better archive handling
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Updates Go version to 1.23.0 and upgrades all dependencies including GitHub API client |
| internal/gh/*.go | Updates GitHub API client import from v44 to v73 |
| internal/gh/assets.go | Adds ARM64 support for Linux platform detection |
| internal/unarchive/unarchive.go | Migrates from archiver/v4 to mholt/archives library |
| CLAUDE.md | Adds new documentation file for Claude Code integration |
| .github/workflows/*.yml | Updates GitHub Actions to newer versions and adds cross-platform testing |
| return err | ||
| } | ||
| return ex.Extract(context.Background(), input, nil, handler) | ||
| return ex.Extract(ctx, input, handler) |
There was a problem hiding this comment.
The Extract method call has changed its signature by removing the third parameter (nil). Verify that this API change is intentional and that the nil parameter was not serving a functional purpose in the previous archiver library.
| ex, ok := format.(archiver.Extractor) | ||
| ex, ok := format.(archives.Extractor) | ||
| if !ok { | ||
| return err |
There was a problem hiding this comment.
This line returns 'err' which was set from the archives.Identify() call on line 26, but it should return a more appropriate error indicating that the format doesn't support extraction. Consider using a descriptive error message like 'fmt.Errorf("format does not support extraction")'.
- Updated .goreleaser.yml to modern conventions: - Removed deprecated nfpms, publishers, and package formats (deb/rpm/apk) - Simplified to only build tarballs for macOS/Linux and zip for Windows - Fixed universal binary creation for macOS - Updated brews config to use v1 syntax for compatibility - Added changelog configuration - Updated .github/workflows/build.yml: - Replaced Task with Just for CI pipeline - Removed package testing job (no longer needed) - Simplified workflow with fewer dependencies - Added snapshot builds for main branch - Updated justfile: - Added 'snapshot' command for local testing - Simplified 'goreleaser' command - Removed package testing commands (no longer needed) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Added version: 2 declaration - Updated brews.repository syntax (from tap to repository) - Updated snapshot.version_template (from name_template) - Configured format_overrides for Windows zip files - Updated CI to use GoReleaser v2 (~> v2) All archives now build correctly: - macOS universal binary (tar.gz) - Linux amd64/arm64 (tar.gz) - Windows amd64/arm64 (zip) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Unit Tests (internal/gh/assets_test.go): - Test OS asset suffix selection (linux/darwin/windows/rhel7) - Test asset finding logic with mock GitHub assets - Test missing asset handling (e.g., ARM64 on older releases) - Runtime-aware tests for ARM64 vs AMD64 detection Integration Tests (internal/gh/integration_test.go): - Test asset availability across Quarto versions (1.2-1.8) - Verify ARM64 Linux support timeline (absent in v1.2, present in v1.3+) - Test real GitHub API calls for release fetching - Optional download tests for actual asset downloads CI Workflow (.github/workflows/integration-tests.yml): - Cross-platform testing on Ubuntu, macOS, and Windows - Separate jobs for fast API tests vs slow download tests - Integration tests run on PR, download tests only on main/manual Test Coverage: ✓ v1.2.335 - no ARM64 Linux (identifies the issue) ✓ v1.4.553 - has ARM64 Linux ✓ v1.5.54 - has ARM64 Linux ✓ v1.7.34 - has ARM64 Linux ✓ v1.8.25 - has ARM64 Linux 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ions Changes to internal/gh/assets.go: - Enhanced DownloadReleaseAsset error messages with specific details - Added architecture info (arm64/amd64) to error output - Created listAvailableAssets() helper to show what's actually available - Error now displays: target OS, architecture, expected suffix, and all available assets Changes to internal/gh/integration_test.go: - Updated tests to expect strict behavior (no fallbacks) - Added unsupportedOnARM64 flag to test cases - v1.2.x correctly expected to fail on ARM64 Linux - Added TestDownloadReleaseAsset_UnsupportedPlatform test - Verifies error message quality and informativeness Example error message: "no release asset found for linux (arm64) with suffix 'linux-arm64.tar.gz'. This version may not support your platform/architecture. Available assets: quarto-1.2.335-linux-amd64.tar.gz, ..." Behavior: ✓ Strict - no silent fallbacks to AMD64 ✓ Clear - users understand exactly what's wrong ✓ Actionable - shows available alternatives ✓ Tested - comprehensive test coverage on ARM64 Quarto v1.2.x and earlier will correctly error on ARM64 Linux, informing users these versions don't support their architecture. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.