Add Gerrit forge adapter#119
Conversation
|
+1 on this feature, fills the single gap that I have for the cli tool, will run a qa on it against my gerrit and provide feedback. |
andrew
left a comment
There was a problem hiding this comment.
Thanks for this, the adapter is well structured and follows the existing gitea/gitlab shape closely. Tests pass locally and pr checkout will work for Gerrit changes for free since #117 already handles refs/-prefixed head refs.
A few things to address before merging, see inline. The main ones are the Code-Review label type, the hardcoded abandon/restore messages, and the gerrit-review.googlesource.com default which doesn't actually work with the auth scheme implemented here.
andrew
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround. Labels, abandon/restore/submit bodies, PathUnescape and the intString cleanup all look good.
Two things from the follow-up commit:
Dropping case "gerrit" from defaultDomainForType means it now falls through to github.com — resolve_test.go and repo_test.go even assert {"gerrit", "github.com"}. That's worse than the googlesource default; forge --type gerrit ... without a host will quietly hit github. Return "" for gerrit and have the callers error when the domain is empty.
The HTMLURL change in convertProject went to the wrong spot. /admin/repos/<encoded> was the correct repo page; the comment was about BlobURL building on top of it. I'd revert HTMLURL to /admin/repos/... and have BlobURL just return repoHTMLURL (no good file-browse URL without gitiles).
|
thanks for catching those, I've reverted the HTMLURL logic back to using the /admin/repos/ endpoint and updated BlobURL to just return the repoHTMLURL . I also fixed the domain resolution so that defaultDomainForType explicitly returns an empty string for gerrit , causing the CLI to correctly error out when a domain isn't provided rather than quietly defaulting to github.com. the test assertions have been updated to reflect this behavior too. |
# Conflicts: # README.md # internal/cli/auth.go # internal/config/config.go
There was a problem hiding this comment.
Pull request overview
This PR adds an initial Gerrit forge adapter to the existing multi-forge library/CLI, including auto-detection and integration into the resolver/client wiring, while explicitly returning ErrNotSupported for unsupported service surfaces.
Changes:
- Adds
gerritas a supportedForgeType, wires it into client registration and resolver builders, and extends CLI/config help text accordingly. - Implements a Gerrit REST client (including XSSI prefix stripping) plus core services: repos/projects, branches, tags, files, and change→PR mappings (diff/comments/submit/abandon/restore + reviews).
- Adds detection via
GET /config/server/versionand introduces stubs returningErrNotSupportedfor non-mappable features.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| types.go | Adds Gerrit forge type constant. |
| README.md | Documents Gerrit support and token format. |
| internal/resolve/resolve.go | Wires Gerrit into forge builders and adjusts domain handling. |
| internal/resolve/resolve_test.go | Extends domain default tests for Gerrit. |
| internal/config/config.go | Updates config type comment to include Gerrit. |
| internal/config/config_test.go | Skips token-cmd execution tests on Windows. |
| internal/cli/root.go | Updates CLI help text to list Gerrit. |
| internal/cli/repo_test.go | Extends domain-from-flags tests for Gerrit. |
| internal/cli/auth.go | Updates auth CLI help text to list Gerrit. |
| gerrit/gerrit.go | Adds Gerrit HTTP client helpers (JSON/text, XSSI strip, auth, utilities). |
| gerrit/repos.go | Implements repo/project get/list/create/edit and tag listing. |
| gerrit/branches.go | Implements branch list/create/delete. |
| gerrit/files.go | Implements file content retrieval (base64 decode). |
| gerrit/changes.go | Implements change mapping to PRs (get/list/diff/comments/submit/abandon/restore). |
| gerrit/reviews.go | Implements review submission + reviewer add/remove. |
| gerrit/url.go | Implements Gerrit URL path parsing into ResourceRef. |
| gerrit/stubs.go | Adds ErrNotSupported stubs for unsupported services. |
| gerrit/gerrit_test.go | Adds focused unit tests for core Gerrit behaviors. |
| detect.go | Adds Gerrit detection probe via /config/server/version. |
| forges_test.go | Adds detection test coverage for Gerrit. |
| forge.go | Adds Gerrit builder hook to Client.RegisterDomain and ForgeBuilders. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Adds initial Gerrit support as a forge adapter.
This maps Gerrit projects, branches, tags, files, and changes into the existing forge interfaces where they fit cleanly. Unsupported Gerrit concepts explicitly return
ErrNotSupported.Closes #12.
What works
gerritforge type and detection via/config/server/versionTests
go test ./...git diff --check