Add cross-forge commit ref resolution - #153
Merged
andrew merged 1 commit intoAug 18, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (4)
gitlab/commits_test.go:12
- Import block is not gofmt-compliant: standard library imports should be grouped before third-party imports, with a blank line separating groups. Currently
forgeis placed among stdlib imports, so gofmt will reorder this block.
import (
"context"
"encoding/json"
"errors"
forge "github.com/git-pkgs/forge"
gitlab/commits.go:11
- Import block is not gofmt-compliant: standard library imports should be grouped before third-party imports, with a blank line separating groups. As written,
forgeis interleaved withnet/http/strings, which will be rewritten by gofmt and may fail formatting checks.
import (
"context"
"errors"
forge "github.com/git-pkgs/forge"
"net/http"
"strings"
gitea/commits.go:11
- Import block is not gofmt-compliant: standard library imports should be grouped before third-party imports, with a blank line separating groups.
forgeis currently mixed into the stdlib section and will be reordered by gofmt.
import (
"context"
"errors"
forge "github.com/git-pkgs/forge"
"net/http"
"strings"
gitea/commits_test.go:12
- Import block is not gofmt-compliant: standard library imports should be grouped before third-party imports, with a blank line separating groups.
forgeis currently placed among stdlib imports and will be moved by gofmt.
import (
"context"
"encoding/json"
"errors"
forge "github.com/git-pkgs/forge"
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
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.
Closes #151.
Problem
#150 added
github.CommitResolverso callers can turn a branch, tag or abbreviated SHA into a full commit SHA without listing every tag on a repository. That resolver only exists on the GitHub backend, so a caller working against GitLab or Gitea/Forgejo has no way to reach the same operation: the sharedForgeinterface has no commit-resolution method and importingforge/githubto resolve a GitLab ref is not an option.Gitea and Forgejo support Actions workflows with similar action references, and other forge-backed package sources need the same immutable ref resolution, so this belongs behind the shared interface.
Change
A shared contract, in the new root
commits.go:added to
ForgeasCommits() CommitService, plus aClient.ResolveCommit(ctx, repoURL, ref)convenience that routes by domain the same wayFetchTagsalready does.The package also exports the pieces every backend would otherwise reimplement, which keeps behavior identical across forges instead of merely similar:
IsFullCommitSHAValidateCommitRefErrCommitRefRequiredsentinelCommitRefErrorresolve owner/repo ref "x": ...ResolvedCommitSHABackend coverage:
Repositories.GetCommitSHA1Commits.GetCommit, with the request context threaded throughgitlab.WithContextGetSingleCommitErrNotSupported, matching what those packages already do forCommitStatusServiceBehavior, uniform across the three implemented backends:
forge.ErrNotFound, wrapped with the repository and ref for contextGitHub refactor:
CommitResolverkeeps its exact public API, so nothing that uses #150 changes. Its body now calls the shared helpers instead of holding a private copy ofisFullCommitSHA, and both it and the newCommits()service delegate to oneresolveCommitfunction, so the two entry points cannot drift apart.Note on the interface change
Adding
Commits()toForgeis a breaking change for any out-of-tree implementer of the interface. I went this way because the issue frames the gap as the shared interface lacking the method, and because a service accessor matches how the other fifteen capabilities are exposed. The alternative is an optional interface type-asserted at the call site, the wayAPIBaseURLProviderworks. Happy to switch if you would rather keepForgeadditive-only.Testing
go build ./...,go vet ./...andgofmt -l .are cleango test -race -count=1 ./...passes on all 12 packagesgo tool golangci-lint run ./...reports 0 issueswindows/amd64andlinux/amd64, since CI runs a three-OS matrixNew tests cover the shared helpers and
Client.ResolveCommitrouting in the root package, plus per-backendhttptestcoverage for GitHub, GitLab and Gitea: a successful resolve, a 404 mapped toErrNotFound, an abbreviated SHA in the response being rejected, empty arguments and a full-SHA input provably making zero requests.Rather than trusting the new tests, I mutation-checked them. Removing the response SHA validation, swapping
ErrNotFoundfor a generic error and disabling the full-SHA short-circuit each produced failures in the root package and in all three backends.