mirror of
https://github.com/tiennm99/tiennm99bot.git
synced 2026-10-11 03:13:46 +00:00
fix(migration): satisfy errcheck on Close + Fprint* in migration toolchain
Pre-existing 9 errcheck violations introduced by 1a8da2d blocked golangci-lint on every CI run after the migration toolchain landed. - Wrap defer Close() in anonymous func to match house style (cf. internal/modules/lolschedule/api_client.go:162). - Mark Fprint*/Fprintln to io.Writer with _, _ = (best-effort writes to os.Stdout / bytes.Buffer; errors not actionable for callers). No behavior change. go vet, golangci-lint, go test ./... all clean locally.
This commit is contained in:
1 parent
dc51978f29
commit
0e6f2d0a1f
5 files changed
+76
-10
No files matched your search
@@ -195,7 +195,7 @@ func runTradingAuditDump(args []string) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer f.Close()
|
||||
defer func() { _ = f.Close() }()
|
||||
enc := json.NewEncoder(f)
|
||||
for _, r := range rows {
|
||||
if err := enc.Encode(r); err != nil {
|
||||
|
||||
@@ -70,7 +70,7 @@ func (c *CloudflareD1Client) Query(ctx context.Context, sql string, params []any
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
raw, err := io.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
|
||||
@@ -119,7 +119,7 @@ func (c *CloudflareKVClient) do(ctx context.Context, method, endpoint string) ([
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
|
||||
@@ -36,22 +36,23 @@ func (r *Report) AddSkippedPolicy(reason string) { r.SkippedPolicy[reason]++ }
|
||||
func (r *Report) AddFailed(prefix string) { r.Failed[prefix]++ }
|
||||
|
||||
// Format writes a human-readable summary. Stable ordering (alphabetical) so
|
||||
// rerun diffs stay clean.
|
||||
// rerun diffs stay clean. Write errors are ignored — callers pass os.Stdout
|
||||
// or *bytes.Buffer, where short writes are not actionable.
|
||||
func (r *Report) Format(w io.Writer) {
|
||||
fmt.Fprintln(w, "Migration report")
|
||||
fmt.Fprintln(w, "================")
|
||||
_, _ = fmt.Fprintln(w, "Migration report")
|
||||
_, _ = fmt.Fprintln(w, "================")
|
||||
writeSection(w, "Imported", r.Imported)
|
||||
writeSection(w, "Skipped (already present)", r.SkippedExisting)
|
||||
writeSection(w, "Skipped (policy)", r.SkippedPolicy)
|
||||
writeSection(w, "Failed", r.Failed)
|
||||
fmt.Fprintf(w, "TOTAL imported=%d skipped_existing=%d skipped_policy=%d failed=%d\n",
|
||||
_, _ = fmt.Fprintf(w, "TOTAL imported=%d skipped_existing=%d skipped_policy=%d failed=%d\n",
|
||||
sum(r.Imported), sum(r.SkippedExisting), sum(r.SkippedPolicy), sum(r.Failed))
|
||||
}
|
||||
|
||||
func writeSection(w io.Writer, label string, m map[string]int) {
|
||||
fmt.Fprintf(w, "\n%s:\n", label)
|
||||
_, _ = fmt.Fprintf(w, "\n%s:\n", label)
|
||||
if len(m) == 0 {
|
||||
fmt.Fprintln(w, " (none)")
|
||||
_, _ = fmt.Fprintln(w, " (none)")
|
||||
return
|
||||
}
|
||||
keys := make([]string, 0, len(m))
|
||||
@@ -60,7 +61,7 @@ func writeSection(w io.Writer, label string, m map[string]int) {
|
||||
}
|
||||
sort.Strings(keys)
|
||||
for _, k := range keys {
|
||||
fmt.Fprintf(w, " %-30s %d\n", k, m[k])
|
||||
_, _ = fmt.Fprintf(w, " %-30s %d\n", k, m[k])
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
# Debug + Fix Report — ci.yml lint failure on commit `1f5f304`
|
||||
|
||||
**Date:** 2026-05-16
|
||||
**Failed run:** https://github.com/tiennm99/miti99bot/actions/runs/25952128154
|
||||
**Workflow:** `.github/workflows/ci.yml` job `go (1.25)` step `golangci-lint`
|
||||
**Status:** DONE — fixes verified locally; awaiting CI re-run on next commit.
|
||||
|
||||
## Root cause
|
||||
|
||||
9 `errcheck` violations introduced by commit `d67517e feat(migration): cf→aws migration toolchain ...` and pushed alongside `39491d1` + my `1f5f304` in the same `git push`. The migration-toolchain commit was the first one to add these unchecked errors; prior commit `75e9360` (docs-only) didn't include them. My deploy-workflow commit `1f5f304` is the first that triggered a CI run after `d67517e` landed, so the lint failure surfaced here even though my change touched zero Go files.
|
||||
|
||||
**Not** a regression caused by the auto-register workflow change.
|
||||
|
||||
## Violations
|
||||
|
||||
| File | Line | Pattern |
|
||||
|---|---|---|
|
||||
| `cmd/migrate_cf_data/main.go` | 198 | `defer f.Close()` |
|
||||
| `internal/migration/cloudflare_d1_client.go` | 73 | `defer resp.Body.Close()` |
|
||||
| `internal/migration/cloudflare_kv_client.go` | 122 | `defer resp.Body.Close()` |
|
||||
| `internal/migration/report.go` | 41,42,47,52,54,63 | `fmt.Fprintln` / `fmt.Fprintf` to `io.Writer` |
|
||||
|
||||
## Fixes applied
|
||||
|
||||
**Close calls** — wrap in anonymous defer with underscore-assignment, matching the established project pattern in `internal/modules/lolschedule/api_client.go:162` and `internal/modules/trading/prices.go:90`:
|
||||
|
||||
```go
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
```
|
||||
|
||||
**Fprint* calls** — explicit underscore-assignment, idiomatic for best-effort writes to `os.Stdout` / `*bytes.Buffer` (the only callers per `grep .Format(`):
|
||||
|
||||
```go
|
||||
_, _ = fmt.Fprintln(w, "Migration report")
|
||||
```
|
||||
|
||||
Also added one comment to `Report.Format` noting the rationale (write errors not actionable for the actual callers).
|
||||
|
||||
## Verification
|
||||
|
||||
| Check | Result |
|
||||
|---|---|
|
||||
| `go vet ./...` | clean |
|
||||
| `golangci-lint run ./...` | `0 issues.` |
|
||||
| `go test -race -count=1 ./...` | all 18 packages pass (incl. `internal/migration`) |
|
||||
| `go build ./...` | clean |
|
||||
|
||||
## Files changed (4 production)
|
||||
|
||||
- `cmd/migrate_cf_data/main.go`
|
||||
- `internal/migration/cloudflare_d1_client.go`
|
||||
- `internal/migration/cloudflare_kv_client.go`
|
||||
- `internal/migration/report.go`
|
||||
|
||||
## Why not just add `//nolint:errcheck`?
|
||||
|
||||
Project has zero `//nolint` directives in `internal/` or `cmd/` (verified by `grep -rn "nolint"` — no hits). Anonymous-defer + `_, _ =` matches the existing house style, so no new lint-suppression convention.
|
||||
|
||||
## Why not change `Report.Format` to return `error`?
|
||||
|
||||
API change for ~6 lines of cosmetic improvement; only 1 production caller (`cmd/migrate_cf_data/main.go:172 — report.Format(os.Stdout)`) which would discard the error anyway since stdout writes are not recoverable. KISS / YAGNI.
|
||||
|
||||
## Unresolved questions
|
||||
|
||||
None.
|
||||
Reference in new issue
Block a user