From c10bb7211718bd895be4b7f42e37041405d751f7 Mon Sep 17 00:00:00 2001 From: claw Date: Tue, 12 May 2026 16:25:32 -0700 Subject: [PATCH] fix: address self-review NIT findings on PR #93 - Add timer.Stop() on happy path in retry loop (idiomatic) - Add concurrency caveat to Client doc comment for SetHTTPClient/SetRetryBackoff - Add explicit 'stale'/'waiting' cases to mapCheckRunStatus --- github/client.go | 4 +++- github/pr.go | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/github/client.go b/github/client.go index 293cc17..3916425 100644 --- a/github/client.go +++ b/github/client.go @@ -66,7 +66,8 @@ func asAPIError(err error) (*APIError, bool) { } // Client interacts with the GitHub API. -// A Client is safe for concurrent use by multiple goroutines. +// A Client is safe for concurrent use by multiple goroutines; +// however, SetHTTPClient and SetRetryBackoff must not be called concurrently with requests. type Client struct { baseURL string token string @@ -145,6 +146,7 @@ func (c *Client) doRequest(ctx context.Context, method, url string, accept strin timer := time.NewTimer(delay) select { case <-timer.C: + timer.Stop() case <-ctx.Done(): timer.Stop() return nil, ctx.Err() diff --git a/github/pr.go b/github/pr.go index c26f061..ec330ae 100644 --- a/github/pr.go +++ b/github/pr.go @@ -220,6 +220,8 @@ func mapCheckRunStatus(conclusion *string, _ string) string { return "failure" case "cancelled", "skipped", "neutral": return "success" // non-blocking + case "stale", "waiting": + return "pending" default: return "pending" }