-
Notifications
You must be signed in to change notification settings - Fork 21
Upgrade GitHub and GitLab API dependencies to latest versions #48
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
Conversation
WalkthroughCI workflow actions were upgraded to v5 and Codecov input renamed from file to files. Go dependencies were updated: google/go-github moved from v30 to v74; GitLab client switched from github.com/xanzy/go-gitlab to gitlab.com/gitlab-org/api/client-go; xz bumped to v0.5.15. Source imports adjusted accordingly. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.2.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/product/migration-guide for migration instructions ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #48 +/- ##
=======================================
Coverage 77.07% 77.07%
=======================================
Files 28 28
Lines 1435 1435
=======================================
Hits 1106 1106
Misses 279 279
Partials 50 50
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:
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (9)
.github/workflows/build.yml (4)
22-22: Checkout v5: good; add least-privilege permissions.Explicitly set minimal permissions for the job.
Example outside this range:
jobs: build: permissions: contents: read
48-55: Codecov v5 switch is correct; set env for tags and prefer OIDC.
- env_vars OS/GO won’t populate unless you export them.
- Use OIDC to avoid secrets not available on PRs from forks.
Apply within this step:
- - name: Code coverage with codecov - uses: codecov/codecov-action@v5 + - name: Code coverage with codecov + uses: codecov/codecov-action@v5 with: env_vars: OS,GO - files: ./coverage.txt + files: ./coverage.txt flags: unittests fail_ci_if_error: false - verbose: true - token: ${{ secrets.CODECOV_TOKEN }} + verbose: true + use_oidc: true + env: + OS: ${{ matrix.os }} + GO: ${{ matrix.go_version }}And outside this range (job/workflow level):
permissions: id-token: write
75-79: Download artifacts: narrow and merge for determinism.Download only coverage artifacts and merge them to one folder.
- - name: Download code coverage results - uses: actions/download-artifact@v5 + - name: Download code coverage results + uses: actions/download-artifact@v5 + with: + pattern: code-coverage-report-* + merge-multiple: true
81-86: Sonar scan v5: OK; consider wiring coverage path.For Go coverage ingestion, pass the report path (downloads to workspace).
- - name: Analyze with SonarCloud - uses: sonarsource/sonarqube-scan-action@v5 + - name: Analyze with SonarCloud + uses: sonarsource/sonarqube-scan-action@v5 + with: + args: > + -Dsonar.go.coverage.reportPaths=**/coverage.txt env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}Note: SONAR_HOST_URL is not required for SonarCloud. (github.com)
github_source.go (3)
44-65: Optional: simplify auth using Client.WithAuthToken (drop oauth2 client plumbing)go-github v74 exposes Client.WithAuthToken, which removes the need to build a custom oauth2 client. This reduces deps and makes both public and Enterprise flows consistent.
Example refactor (illustrative only):
- hc := newHTTPClient(token) - - if config.EnterpriseBaseURL == "" - { - // public (or private) repository on standard GitHub offering - client := github.NewClient(hc) - return &GitHubSource{ api: client }, nil - } + var client *github.Client + if config.EnterpriseBaseURL == "" { + client = github.NewClient(nil) + } else { + u := config.EnterpriseUploadURL + if u == "" { + u = config.EnterpriseBaseURL + } + c, err := github.NewEnterpriseClient(config.EnterpriseBaseURL, u, nil) + if err != nil { + return nil, fmt.Errorf("cannot parse GitHub enterprise URL: %w", err) + } + client = c + } + if token != "" { + client = client.WithAuthToken(token) + } return &GitHubSource{ - api: client, + api: client, }, nilNote: if you take this path, remove the oauth2 import and the now-unused newHTTPClient helper.
(github.com)
67-88: “ListReleases returns all available releases” — currently only returns first pageTo fulfill the comment/contract, paginate through all pages.
- rels, res, err := s.api.Repositories.ListReleases(ctx, owner, repo, nil) + opt := &github.ListOptions{PerPage: 100} + var relsAll []*github.RepositoryRelease + for { + rels, res, err := s.api.Repositories.ListReleases(ctx, owner, repo, opt) + if err != nil { + if res != nil && res.StatusCode == http.StatusNotFound { + log.Print("Repository or release not found") + return nil, nil + } + log.Printf("API returned an error response: %s", err) + return nil, err + } + relsAll = append(relsAll, rels...) + if res.NextPage == 0 { + break + } + opt.Page = res.NextPage + } - if err != nil { - if res != nil && res.StatusCode == http.StatusNotFound { - // repository not found or release not found. It's not an error here. - log.Print("Repository or release not found") - return nil, nil - } - log.Printf("API returned an error response: %s", err) - return nil, err - } - releases := make([]SourceRelease, len(rels)) - for i, rel := range rels { + releases := make([]SourceRelease, len(relsAll)) + for i, rel := range relsAll { releases[i] = NewGitHubRelease(rel) } return releases, nilPagination approach per upstream docs (ListOptions + NextPage). (github.com)
100-107: Add an HTTP timeout when following asset redirectsUsing http.DefaultClient risks hanging downloads. Set a bounded timeout.
- client := http.DefaultClient + client := &http.Client{Timeout: 60 * time.Second}Add to imports:
import "time"gitlab_source.go (2)
88-114: Harden asset download: add timeout and status checkDefault client has no timeouts, and non-2xx responses currently flow to callers.
- client := http.DefaultClient + client := &http.Client{Timeout: 60 * time.Second} @@ - response, err := client.Do(req) + response, err := client.Do(req) if err != nil { log.Print(err) return nil, err } - - return response.Body, nil + if response.StatusCode < 200 || response.StatusCode >= 300 { + // ensure body is closed on error paths + _ = response.Body.Close() + return nil, fmt.Errorf("download failed: status %d for %s", response.StatusCode, downloadUrl) + } + return response.Body, nilAdd to imports:
import "time"
96-105: Note on GitLab tokened downloads for release linksSome release “links” resolve to uploads URLs that historically didn’t accept PRIVATE-TOKEN for direct download (redirects to sign-in). If you hit 302/404 on private assets, consider storing artifacts in Generic Packages and linking those, which do support token headers reliably.
References: discussion of uploads behavior and a working Packages approach. (gitlab.com, stackoverflow.com, docs.gitlab.com)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
.github/workflows/build.yml(3 hunks)github_release.go(1 hunks)github_source.go(1 hunks)gitlab_release.go(1 hunks)gitlab_source.go(1 hunks)go.mod(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and test (1.24, macos-latest)
- GitHub Check: Build and test (1.24, windows-latest)
🔇 Additional comments (8)
go.mod (1)
8-11: Manual API compatibility review required
The project builds successfully with the updated dependencies (go-github v74, GitLab client v0.142.6, xz v0.5.15) and no missing symbols were detected. However, the major version bump of the GitHub client (v30→v74) and the GitLab client migration may include behavioral or API surface changes—please manually review all critical API usages for breaking changes.gitlab_release.go (1)
6-6: LGTM - Consistent migration to official GitLab client.The import change aligns with the package migration and maintains the same alias and API usage patterns as
gitlab_source.go.github_release.go (1)
6-6: LGTM - GitHub client upgrade properly implemented.The version upgrade from v30 to v74 is consistent with the change in
github_source.goand maintains identical API usage patterns..github/workflows/build.yml (1)
70-74: Checkout v5 in Sonar job: looks good.github_source.go (2)
10-10: Go-GitHub import bumped to v74 — looks goodImport path matches the latest major-versioned module and aligns with upstream guidance. No other changes required here.
3-12: Imports are up-to-date: All go-github imports use v74 and there are no xanzy/go-gitlab imports remaining.gitlab_source.go (2)
10-10: Switch to official GitLab client-go import — looks correctNew import path and alias are accurate for gitlab.com/gitlab-org/api/client-go.
61-70: LGTM on ListReleases call shapeSignature matches client-go: ListReleases(pid any, opt *ListReleasesOptions, options ...RequestOptionFunc). Passing nil options with gitlab.WithContext(ctx) is valid.
Upgrade GitHub and GitLab API dependencies to latest versions
This PR updates the project's core dependencies to their latest stable versions:
Dependency Updates:
github.com/google/go-github/v30→v74.0.0github.com/xanzy/go-gitlab→gitlab.com/gitlab-org/api/client-go v0.142.6github.com/ulikunitz/xz v0.5.14→v0.5.15CI/CD Updates:
actions/checkout@v4→v5codecov/codecov-action@v4→v5sonarsource/sonarqube-scan-action@v4→v5These updates ensure compatibility with the latest API features and security patches while maintaining backward compatibility for existing functionality.
Summary by CodeRabbit