From 5252143a33c634301efea27789016366d3548853 Mon Sep 17 00:00:00 2001 From: claw Date: Wed, 13 May 2026 09:20:33 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20address=20review=20feedback=20=E2=80=94?= =?UTF-8?q?=20alias=20default,=20acronym=20convention,=20observability?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - #19639: Use empty default for --gitea-url alias to remove ordering dependency - #19640: Upgrade slog.Warn to slog.Error for missing ReviewSuperseder (signals bug) - #19641: Remove orphaned comment fragment from buildSupersededBody relocation - #19642: Rename ProviderGithub → ProviderGitHub per Go acronym convention - #19643: Log resolution failures at debug level in SupersedeReviews --- cmd/review-bot/main.go | 10 ++++------ gitea/adapter.go | 4 +++- vcs/provider.go | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index f08deb8..bc1f192 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -85,8 +85,8 @@ func main() { aicoreResourceGroup := flag.String("aicore-resource-group", envOrDefault("AICORE_RESOURCE_GROUP", "default"), "SAP AI Core resource group (for provider=aicore)") // Backward-compatible alias: --gitea-url shares vcsURL's pointer (last flag wins). - // Must stay after vcsURL declaration and before flag.Parse(). - flag.StringVar(vcsURL, "gitea-url", *vcsURL, "Deprecated: use --vcs-url instead") + // Shares vcsURL pointer; empty default avoids ordering dependency with vcsURL declaration. + flag.StringVar(vcsURL, "gitea-url", "", "Deprecated: use --vcs-url instead") flag.Parse() @@ -162,7 +162,7 @@ func main() { case vcs.ProviderGitea: giteaClient := gitea.NewClient(*vcsURL, *reviewerToken) client = gitea.NewAdapter(giteaClient) - case vcs.ProviderGithub: + case vcs.ProviderGitHub: client = github.NewClient(*reviewerToken, *baseURL) default: panic("unreachable: provider validation should have caught " + vcsProvider.String()) @@ -501,7 +501,7 @@ func main() { os.Exit(1) } } else { - slog.Warn("provider does not support review superseding", "provider", vcsProvider) + slog.Error("provider does not support review superseding", "provider", vcsProvider) } } } @@ -721,8 +721,6 @@ func validateWorkspacePath(path, pathName string) (string, error) { return resolvedPath, nil } -// with collapsed original content and the commit it was evaluated against. - // hasSharedToken detects if another review-bot role posted under the same // VCS user. This indicates misconfiguration where two roles share a token // instead of having separate accounts. Returns true if shared token diff --git a/gitea/adapter.go b/gitea/adapter.go index 9c4f753..176c722 100644 --- a/gitea/adapter.go +++ b/gitea/adapter.go @@ -282,7 +282,9 @@ func (a *Adapter) SupersedeReviews(ctx context.Context, owner, repo string, prNu if c.ID == 0 { continue } - _ = underlying.ResolveComment(ctx, owner, repo, c.ID) + if err := underlying.ResolveComment(ctx, owner, repo, c.ID); err != nil { + slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err) + } } } return nil diff --git a/vcs/provider.go b/vcs/provider.go index 2d19409..0d9819b 100644 --- a/vcs/provider.go +++ b/vcs/provider.go @@ -7,13 +7,13 @@ type VCSProvider string const ( ProviderGitea VCSProvider = "gitea" - ProviderGithub VCSProvider = "github" + ProviderGitHub VCSProvider = "github" ) // Valid reports whether p is a known VCS provider. func (p VCSProvider) Valid() bool { switch p { - case ProviderGitea, ProviderGithub: + case ProviderGitea, ProviderGitHub: return true default: return false