Skip to content

Commit b3636eb

Browse files
authored
Merge pull request #70 from tstromberg/sprinkler
update libs, improve linting
2 parents 290abb4 + 3dacdec commit b3636eb

22 files changed

+328
-274
lines changed

.golangci.yml

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ linters:
4040
- gochecknoglobals # checks that no global variables exist
4141
- cyclop # replaced by revive
4242
- gocyclo # replaced by revive
43+
- forbidigo # needs configuration to be useful
4344
- funlen # replaced by revive
4445
- godox # TODO's are OK
4546
- ireturn # It's OK
@@ -151,10 +152,10 @@ linters:
151152

152153
nakedret:
153154
# Default: 30
154-
max-func-lines: 4
155+
max-func-lines: 7
155156

156157
nestif:
157-
min-complexity: 12
158+
min-complexity: 15
158159

159160
nolintlint:
160161
# Exclude following linters from requiring an explanation.
@@ -170,17 +171,11 @@ linters:
170171
rules:
171172
- name: add-constant
172173
severity: warning
173-
disabled: false
174-
exclude: [""]
175-
arguments:
176-
- max-lit-count: "5"
177-
allow-strs: '"","\n"'
178-
allow-ints: "0,1,2,3,24,30,365,1024,0o600,0o700,0o750,0o755"
179-
allow-floats: "0.0,0.,1.0,1.,2.0,2."
174+
disabled: true
180175
- name: cognitive-complexity
181-
arguments: [55]
176+
disabled: true # prefer maintidx
182177
- name: cyclomatic
183-
arguments: [60]
178+
disabled: true # prefer maintidx
184179
- name: function-length
185180
arguments: [150, 225]
186181
- name: line-length-limit
@@ -212,8 +207,14 @@ linters:
212207
os-temp-dir: true
213208

214209
varnamelen:
215-
max-distance: 40
210+
max-distance: 75
216211
min-name-length: 2
212+
check-receivers: false
213+
ignore-names:
214+
- r
215+
- w
216+
- f
217+
- err
217218

218219
exclusions:
219220
# Default: []

Makefile

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ LINTERS :=
204204
FIXERS :=
205205

206206
GOLANGCI_LINT_CONFIG := $(LINT_ROOT)/.golangci.yml
207-
GOLANGCI_LINT_VERSION ?= v2.3.1
207+
GOLANGCI_LINT_VERSION ?= v2.5.0
208208
GOLANGCI_LINT_BIN := $(LINT_ROOT)/out/linters/golangci-lint-$(GOLANGCI_LINT_VERSION)-$(LINT_ARCH)
209209
$(GOLANGCI_LINT_BIN):
210210
mkdir -p $(LINT_ROOT)/out/linters
@@ -234,9 +234,19 @@ yamllint-lint: $(YAMLLINT_BIN)
234234
PYTHONPATH=$(YAMLLINT_ROOT)/dist $(YAMLLINT_ROOT)/dist/bin/yamllint .
235235

236236
.PHONY: _lint $(LINTERS)
237-
_lint: $(LINTERS)
237+
_lint:
238+
@exit_code=0; \
239+
for target in $(LINTERS); do \
240+
$(MAKE) $$target || exit_code=1; \
241+
done; \
242+
exit $$exit_code
238243

239244
.PHONY: fix $(FIXERS)
240-
fix: $(FIXERS)
245+
fix:
246+
@exit_code=0; \
247+
for target in $(FIXERS); do \
248+
$(MAKE) $$target || exit_code=1; \
249+
done; \
250+
exit $$exit_code
241251

242252
# END: lint-install .

cmd/goose/browser_rate_limiter.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// Package main implements browser rate limiting for PR auto-open feature.
21
package main
32

43
import (

cmd/goose/cache.go

Lines changed: 68 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// Package main - cache.go provides caching functionality for Turn API responses.
21
package main
32

43
import (
@@ -23,6 +22,70 @@ type cacheEntry struct {
2322
UpdatedAt time.Time `json:"updated_at"`
2423
}
2524

25+
// checkCache checks the cache for a PR and returns the cached data if valid.
26+
// Returns (cachedData, cacheHit, hasRunningTests).
27+
func (app *App) checkCache(cacheFile, url string, updatedAt time.Time) (cachedData *turn.CheckResponse, cacheHit bool, hasRunningTests bool) {
28+
fileData, readErr := os.ReadFile(cacheFile)
29+
if readErr != nil {
30+
if !os.IsNotExist(readErr) {
31+
slog.Debug("[CACHE] Cache file read error", "url", url, "error", readErr)
32+
}
33+
return nil, false, false
34+
}
35+
36+
var entry cacheEntry
37+
if unmarshalErr := json.Unmarshal(fileData, &entry); unmarshalErr != nil {
38+
slog.Warn("Failed to unmarshal cache data", "url", url, "error", unmarshalErr)
39+
// Remove corrupted cache file
40+
if removeErr := os.Remove(cacheFile); removeErr != nil {
41+
slog.Error("Failed to remove corrupted cache file", "error", removeErr)
42+
}
43+
return nil, false, false
44+
}
45+
46+
// Check if cache is expired or PR updated
47+
if time.Since(entry.CachedAt) >= cacheTTL || !entry.UpdatedAt.Equal(updatedAt) {
48+
// Log why cache was invalid
49+
if !entry.UpdatedAt.Equal(updatedAt) {
50+
slog.Debug("[CACHE] Cache miss - PR updated",
51+
"url", url,
52+
"cached_pr_time", entry.UpdatedAt.Format(time.RFC3339),
53+
"current_pr_time", updatedAt.Format(time.RFC3339))
54+
} else {
55+
slog.Debug("[CACHE] Cache miss - TTL expired",
56+
"url", url,
57+
"cached_at", entry.CachedAt.Format(time.RFC3339),
58+
"cache_age", time.Since(entry.CachedAt).Round(time.Second),
59+
"ttl", cacheTTL)
60+
}
61+
return nil, false, false
62+
}
63+
64+
// Check for incomplete tests that should invalidate cache
65+
cacheAge := time.Since(entry.CachedAt)
66+
testState := entry.Data.PullRequest.TestState
67+
isTestIncomplete := testState == "running" || testState == "queued" || testState == "pending"
68+
if entry.Data != nil && isTestIncomplete && cacheAge < runningTestsCacheBypass {
69+
slog.Debug("[CACHE] Cache invalidated - tests incomplete and cache entry is fresh",
70+
"url", url,
71+
"test_state", testState,
72+
"cache_age", cacheAge.Round(time.Minute),
73+
"cached_at", entry.CachedAt.Format(time.RFC3339))
74+
return nil, false, true
75+
}
76+
77+
// Cache hit
78+
slog.Debug("[CACHE] Cache hit",
79+
"url", url,
80+
"cached_at", entry.CachedAt.Format(time.RFC3339),
81+
"cache_age", time.Since(entry.CachedAt).Round(time.Second),
82+
"pr_updated_at", entry.UpdatedAt.Format(time.RFC3339))
83+
if app.healthMonitor != nil {
84+
app.healthMonitor.recordCacheAccess(true)
85+
}
86+
return entry.Data, true, false
87+
}
88+
2689
// turnData fetches Turn API data with caching.
2790
func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (*turn.CheckResponse, bool, error) {
2891
hasRunningTests := false
@@ -45,57 +108,10 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
45108

46109
// Skip cache if --no-cache flag is set
47110
if !app.noCache {
48-
// Try to read from cache (gracefully handle all cache errors)
49-
if data, readErr := os.ReadFile(cacheFile); readErr == nil {
50-
var entry cacheEntry
51-
if unmarshalErr := json.Unmarshal(data, &entry); unmarshalErr != nil {
52-
slog.Warn("Failed to unmarshal cache data", "url", url, "error", unmarshalErr)
53-
// Remove corrupted cache file
54-
if removeErr := os.Remove(cacheFile); removeErr != nil {
55-
slog.Error("Failed to remove corrupted cache file", "error", removeErr)
56-
}
57-
} else if time.Since(entry.CachedAt) < cacheTTL && entry.UpdatedAt.Equal(updatedAt) {
58-
// Check if cache is still valid (10 day TTL, but PR UpdatedAt is primary check)
59-
// But invalidate cache for PRs with incomplete tests if cache entry is fresh (< 90 minutes old)
60-
cacheAge := time.Since(entry.CachedAt)
61-
testState := entry.Data.PullRequest.TestState
62-
isTestIncomplete := testState == "running" || testState == "queued" || testState == "pending"
63-
if entry.Data != nil && isTestIncomplete && cacheAge < runningTestsCacheBypass {
64-
hasRunningTests = true
65-
slog.Debug("[CACHE] Cache invalidated - tests incomplete and cache entry is fresh",
66-
"url", url,
67-
"test_state", testState,
68-
"cache_age", cacheAge.Round(time.Minute),
69-
"cached_at", entry.CachedAt.Format(time.RFC3339))
70-
// Don't return cached data - fall through to fetch fresh data with current time
71-
} else {
72-
slog.Debug("[CACHE] Cache hit",
73-
"url", url,
74-
"cached_at", entry.CachedAt.Format(time.RFC3339),
75-
"cache_age", time.Since(entry.CachedAt).Round(time.Second),
76-
"pr_updated_at", entry.UpdatedAt.Format(time.RFC3339))
77-
if app.healthMonitor != nil {
78-
app.healthMonitor.recordCacheAccess(true)
79-
}
80-
return entry.Data, true, nil
81-
}
82-
} else {
83-
// Log why cache was invalid
84-
if !entry.UpdatedAt.Equal(updatedAt) {
85-
slog.Debug("[CACHE] Cache miss - PR updated",
86-
"url", url,
87-
"cached_pr_time", entry.UpdatedAt.Format(time.RFC3339),
88-
"current_pr_time", updatedAt.Format(time.RFC3339))
89-
} else if time.Since(entry.CachedAt) >= cacheTTL {
90-
slog.Debug("[CACHE] Cache miss - TTL expired",
91-
"url", url,
92-
"cached_at", entry.CachedAt.Format(time.RFC3339),
93-
"cache_age", time.Since(entry.CachedAt).Round(time.Second),
94-
"ttl", cacheTTL)
95-
}
96-
}
97-
} else if !os.IsNotExist(readErr) {
98-
slog.Debug("[CACHE] Cache file read error", "url", url, "error", readErr)
111+
if cachedData, cacheHit, runningTests := app.checkCache(cacheFile, url, updatedAt); cacheHit {
112+
return cachedData, true, nil
113+
} else if runningTests {
114+
hasRunningTests = true
99115
}
100116
}
101117

cmd/goose/github.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// Package main - github.go contains GitHub API integration functions.
21
package main
32

43
import (
@@ -62,7 +61,7 @@ func (app *App) initClients(ctx context.Context) error {
6261
// initSprinklerOrgs fetches the user's organizations and starts sprinkler monitoring.
6362
func (app *App) initSprinklerOrgs(ctx context.Context) error {
6463
if app.client == nil || app.sprinklerMonitor == nil {
65-
return fmt.Errorf("client or sprinkler not initialized")
64+
return errors.New("client or sprinkler not initialized")
6665
}
6766

6867
// Get current user
@@ -74,7 +73,7 @@ func (app *App) initSprinklerOrgs(ctx context.Context) error {
7473
user = app.targetUser
7574
}
7675
if user == "" {
77-
return fmt.Errorf("no user configured")
76+
return errors.New("no user configured")
7877
}
7978

8079
slog.Info("[SPRINKLER] Fetching user's organizations", "user", user)
@@ -136,7 +135,7 @@ func (app *App) initSprinklerOrgs(ctx context.Context) error {
136135
// Update sprinkler with all orgs at once
137136
if len(allOrgs) > 0 {
138137
app.sprinklerMonitor.updateOrgs(allOrgs)
139-
if err := app.sprinklerMonitor.start(); err != nil {
138+
if err := app.sprinklerMonitor.start(ctx); err != nil {
140139
return fmt.Errorf("start sprinkler: %w", err)
141140
}
142141
}
@@ -287,7 +286,13 @@ func (app *App) executeGitHubQuery(ctx context.Context, query string, opts *gith
287286
return result, err
288287
}
289288

290-
func (app *App) executeGitHubQueryInternal(ctx context.Context, query string, opts *github.SearchOptions, result **github.IssuesSearchResult, resp **github.Response) error {
289+
func (app *App) executeGitHubQueryInternal(
290+
ctx context.Context,
291+
query string,
292+
opts *github.SearchOptions,
293+
result **github.IssuesSearchResult,
294+
resp **github.Response,
295+
) error {
291296
return retry.Do(func() error {
292297
// Create timeout context for GitHub API call
293298
githubCtx, cancel := context.WithTimeout(ctx, 30*time.Second)

cmd/goose/icons.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,17 @@ const (
2424
// getIcon returns the icon bytes for the given type.
2525
func getIcon(iconType IconType) []byte {
2626
switch iconType {
27-
case IconGoose:
27+
case IconGoose, IconBoth:
28+
// For both, we'll use the goose icon as primary
2829
return iconGoose
2930
case IconPopper:
3031
return iconPopper
3132
case IconCockroach:
3233
return iconCockroach
33-
case IconSmiling:
34-
return iconSmiling
3534
case IconWarning:
3635
return iconWarning
3736
case IconLock:
3837
return iconLock
39-
case IconBoth:
40-
// For both, we'll use the goose icon as primary
41-
return iconGoose
4238
default:
4339
return iconSmiling
4440
}
@@ -47,7 +43,7 @@ func getIcon(iconType IconType) []byte {
4743
// setTrayIcon updates the system tray icon based on PR counts.
4844
func (app *App) setTrayIcon(iconType IconType) {
4945
iconBytes := getIcon(iconType)
50-
if iconBytes == nil || len(iconBytes) == 0 {
46+
if len(iconBytes) == 0 {
5147
slog.Warn("Icon bytes are empty, skipping icon update", "type", iconType)
5248
return
5349
}

cmd/goose/loginitem_darwin.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
//go:build darwin
22

3-
// Package main - loginitem_darwin.go provides macOS-specific login item management.
43
package main
54

65
import (

0 commit comments

Comments
 (0)