refactor(github): extract doRequestCore, address review feedback
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 28s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 26s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 55s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m28s

- MAJOR: Extract doRequestCore to eliminate doRequest/doRequestWithBody
  duplication. Both now delegate to a shared implementation with the
  retry/backoff logic in a single place.

- MINOR: Replace custom containsStr/containsSubstring helpers with
  strings.Contains in review_test.go.

- MINOR: Use http.Method* constants (MethodPost, MethodDelete, MethodPut)
  in review.go for consistency with doGet.

- MINOR: Remove redundant APPROVED/DISMISSED cases from
  translateGitHubReviewState that were identical to the default passthrough.

- NIT: Clarify DeleteReview comment about COMMENTED being a GitHub API
  state name.

- DismissReview Event field verified as required by GitHub API docs;
  kept as-is.
This commit is contained in:
claw
2026-05-13 00:35:59 -07:00
parent 118e9df758
commit 84cc23c7e9
3 changed files with 54 additions and 135 deletions
+2 -13
View File
@@ -7,6 +7,7 @@ import (
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
@@ -153,7 +154,7 @@ func TestPostReview_MalformedResponse(t *testing.T) {
if err == nil {
t.Fatal("expected error for malformed response")
}
if !containsStr(err.Error(), "parse review response") {
if !strings.Contains(err.Error(), "parse review response") {
t.Errorf("expected parse error, got: %v", err)
}
}
@@ -379,16 +380,4 @@ func TestTranslateGitHubReviewState(t *testing.T) {
}
}
// containsStr is a test helper for checking error messages.
func containsStr(s, substr string) bool {
return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsSubstring(s, substr))
}
func containsSubstring(s, sub string) bool {
for i := 0; i <= len(s)-len(sub); i++ {
if s[i:i+len(sub)] == sub {
return true
}
}
return false
}