Skip to content

refactor: resolve golangci-lint violations and add linter configuration#55

Merged
flexiondotorg merged 6 commits intomainfrom
lint
Mar 14, 2026
Merged

refactor: resolve golangci-lint violations and add linter configuration#55
flexiondotorg merged 6 commits intomainfrom
lint

Conversation

@flexiondotorg
Copy link
Contributor

Summary

Integrate linting into the development workflow by adding comprehensive golangci-lint configuration, resolving linter violations across the codebase, and updating documentation and CI/CD to reflect these changes. This establishes consistent code quality standards and prevents future violations.

Changes

  • Add golangci-lint configuration (.golangci.yml) with tailored rules
  • Resolve linting violations across internal packages (logging, processor, ui, audio, cli)
  • Refactor logging to use zero values instead of math.NaN() for uninitialized variables
  • Update CI/CD workflow to quote shell variables in GitHub Actions
  • Update development tooling (justfile, flake.nix, .envrc)
  • Document agent responsibilities and testing guidelines in AGENTS.md

Testing

  • Ran golangci-lint across all Go files to verify no violations remain
  • Tested refactored code paths to ensure functional equivalence
  • Verified CI/CD workflow syntax after shell variable quoting

- Add .golangci.yml with 13 enabled linters (errorlint, gosec,
  staticcheck, etc.)
- Add lint recipe to justfile (go vet, gocyclo, ineffassign,
  golangci-lint, actionlint)
- Add development dependencies: actionlint, gocyclo, golangci-lint,
  ineffassign
- Update .envrc to use nix_direnv_manual_reload with explicit flake
  reference
- Clean up trailing whitespace in justfile release recipe

Signed-off-by: Martin Wimpress <code@wimpress.io>
…lized variables

Replace ineffective assignments of math.NaN() to float64 variables with
explicit zero-value declarations. Variables are conditionally assigned
within if blocks, so initialization to NaN was redundant.

  - inputRMS: 0.0 instead of math.NaN()
  - inputPeak: 0.0 instead of math.NaN()
  - inputCrest: 0.0 instead of math.NaN()

Signed-off-by: Martin Wimpress <code@wimpress.io>
Quote $GITHUB_OUTPUT and $PREV_TAG to satisfy actionlint linting rules
and avoid issues with variable expansion in shell contexts.

Signed-off-by: Martin Wimpress <code@wimpress.io>
- Convert if-else chains to switch statements (gocritic)
- Replace compound assignments with shorthand operators: %=, /=
  (gocritic assignOp)
- Replace fmt.Sprintf with fmt.Fprintf (staticcheck QF1012)
- Convert int loops to range-based iteration (modernize rangeint)
- Align struct fields for readability (gofumpt)
- Fix variable naming from snake_case to camelCase (revive var-naming)
- Add //nolint comments for false positives (gosec G115, gocritic)
- Remove unused functions and parameters (unparam, unusedfunc)
- Fix other errcheck and formatting violations

Signed-off-by: Martin Wimpress <code@wimpress.io>
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 30 files

Confidence score: 4/5

  • Overall risk looks low to moderate; the issues are mid‑severity setup/robustness concerns rather than clear runtime regressions.
  • Most impactful is in internal/processor/encoder.go: ignoring AVOptSetInt errors could mask encoder misconfiguration and make failures harder to diagnose.
  • There are also workflow consistency concerns in justfile and .envrc that could lead to inconsistent dev setup checks or stale toolchain changes.
  • Pay close attention to internal/processor/encoder.go, justfile, .envrc - error handling and dev setup consistency.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="justfile">

<violation number="1" location="justfile:169">
P2: Make `lint` depend on `_check-submodule` so it fails with the same setup guard as `build` and `test`.</violation>
</file>

<file name=".envrc">

<violation number="1" location=".envrc:1">
P2: Avoid manual reload mode here; it leaves flake/toolchain changes unapplied until developers run `nix-direnv-reload`.</violation>
</file>

<file name="internal/processor/encoder.go">

<violation number="1" location="internal/processor/encoder.go:89">
P2: Handle `AVOptSetInt` errors instead of discarding them during encoder setup.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

…ange-over-int

- Replace interface{} with any builtin type alias
- Use max/min builtins instead of conditional assignments
- Replace for i := 0; i < len(...) with for i := range loops
- Improve error handling in FLAC encoder setup
- Add submodule check to lint recipe

Signed-off-by: Martin Wimpress <code@wimpress.io>
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 4 files (changes from recent commits).

Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.

@flexiondotorg flexiondotorg merged commit 4746c7f into main Mar 14, 2026
7 checks passed
@flexiondotorg flexiondotorg deleted the lint branch March 14, 2026 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant