Skip to content

Add golangci-lint integration and fix all linting issues#51

Merged
wham merged 7 commits intomainfrom
copilot/add-linter-to-go-code
Dec 23, 2025
Merged

Add golangci-lint integration and fix all linting issues#51
wham merged 7 commits intomainfrom
copilot/add-linter-to-go-code

Conversation

Copy link
Contributor

Copilot AI commented Dec 21, 2025

Integrates golangci-lint into the local development build script using default configuration to provide code quality feedback during development, and fixes all existing linting issues in the codebase.

Changes

  • scripts/run: Added golangci-lint execution before build (non-blocking in development) and CGO flags (CGO_ENABLED=1 and CGO_LDFLAGS="-lm") to match CI configuration
  • main.md: Added Code Quality section documenting linter usage and CI/local behavior differences
  • main.go: Fixed all 11 pre-existing linting issues (8 errcheck + 3 staticcheck)

Background

The linter runs automatically before each build via scripts/run, providing immediate feedback on code quality issues while allowing development to continue (non-blocking). The CGO flags prevent build failures with undefined reference errors (e.g., undefined reference to log) and align local builds with CI configuration.

Linting Issues Fixed

All 11 linting issues found in the codebase have been resolved:

  • 8 errcheck issues: Added proper error checking for all Close() operations (database connections, SQL rows, HTTP response bodies) using deferred anonymous functions that check and log close errors
  • 3 staticcheck issues:
    • Removed redundant type declaration (ST1023)
    • Applied De Morgan's law to simplify logic (QF1001)
    • Fixed error message capitalization (ST1005)

The codebase now passes golangci-lint with 0 issues.

Integration

  • Local Development: ./scripts/run [command] runs linter with warnings but continues build to allow rapid iteration
  • CI: .github/workflows/build.yml runs golangci-lint with its default configuration (5 standard linters: errcheck, govet, ineffassign, staticcheck, unused) and only reports new issues introduced in PRs (using --new-from-rev flag)
  • No custom configuration: Uses golangci-lint's out-of-box default linters without a .golangci.yml file
# Local development build with linting
./scripts/run [command]

# Standalone linting
golangci-lint run --timeout=5m
Original prompt

Add linter for the Go code. Update the spec too. Use the most standard thing and most common rules. Make part of the build task and run script.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits December 21, 2025 17:49
Co-authored-by: wham <448809+wham@users.noreply.github.com>
Co-authored-by: wham <448809+wham@users.noreply.github.com>
Co-authored-by: wham <448809+wham@users.noreply.github.com>
Copilot AI changed the title [WIP] Add linter for Go code and update spec Add golangci-lint configuration for Go code quality Dec 21, 2025
Copilot AI requested a review from wham December 21, 2025 17:58
@wham wham requested a review from Copilot December 22, 2025 01:46
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds golangci-lint configuration to enforce code quality standards across the Go codebase. The implementation includes comprehensive linter rules, integration with both local development workflow and CI pipeline, and clear documentation.

Key changes:

  • Configured golangci-lint with 11 standard linters including error checking, static analysis, security scanning, and complexity checks
  • Integrated linting into the build script with non-blocking behavior for local development
  • Documented linter configuration, enabled rules, and CI/local behavior differences

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
.golangci.yml Configures 11 linters with appropriate settings and test file exclusions
scripts/run Adds non-blocking linter execution before builds and updates CGO flags
main.md Documents the linting setup, enabled linters, usage, and CI integration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


# Build with FTS5 support enabled
CGO_CFLAGS="-DSQLITE_ENABLE_FTS5" go build -gcflags="all=-N -l" -o ./build/github-brain .
CGO_ENABLED=1 CGO_CFLAGS="-DSQLITE_ENABLE_FTS5" CGO_LDFLAGS="-lm" go build -gcflags="all=-N -l" -o ./build/github-brain .
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The addition of CGO_ENABLED=1 and CGO_LDFLAGS flags appears unrelated to the linting changes described in the PR. This build configuration change should either be mentioned in the PR description or separated into a different PR for clarity.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CGO flags were added to align scripts/run with the CI build configuration (see .github/workflows/build.yml:29). Without CGO_LDFLAGS="-lm", the local build fails with undefined reference to log after linting. This ensures developers can run the full lint+build workflow locally matching CI behavior.

@wham
Copy link
Owner

wham commented Dec 23, 2025

@copilot Is the config file needed, can we work with out-of-box configuration?

Copy link
Contributor Author

Copilot AI commented Dec 23, 2025

@copilot Is the config file needed, can we work with out-of-box configuration?

The config file adds 6 useful linters beyond the default 5. Out-of-box only enables: errcheck, govet, ineffassign, staticcheck, unused. Our config adds: revive (style), misspell, gosec (security), bodyclose, goconst, gocyclo (complexity). It also customizes settings like excluding G104 from gosec (covered by errcheck) and setting complexity threshold to 15. Without the config, you'd miss important checks like security issues, HTTP body leaks, and code complexity.

@wham
Copy link
Owner

wham commented Dec 23, 2025

@copilot Let's go with the standard set of linters. I'm fine with it.

@wham
Copy link
Owner

wham commented Dec 23, 2025

@copilot OK, no Additional common linters. Let's go without the config file. Remove it, and update the spec too.

…ags fix

Co-authored-by: wham <448809+wham@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 23, 2025

@copilot OK, no Additional common linters. Let's go without the config file. Remove it, and update the spec too.

Done in commit 7f99edd. Removed .golangci.yml config file, removed Code Quality section from main.md, and removed linting from scripts/run. The CI will now use golangci-lint's default configuration (5 standard linters). Kept the CGO flags fix (CGO_ENABLED=1 and CGO_LDFLAGS="-lm") to align with CI build configuration.

Copilot AI changed the title Add golangci-lint configuration for Go code quality Fix CGO build flags in scripts/run to match CI configuration Dec 23, 2025
@wham
Copy link
Owner

wham commented Dec 23, 2025

@copilot Wait, I only wanted to remove the config file. Keep the script/run integration and lightweight spec.

…ain.md

Co-authored-by: wham <448809+wham@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 23, 2025

@copilot Wait, I only wanted to remove the config file. Keep the script/run integration and lightweight spec.

Done in commit 58a1927. Added back linting integration to scripts/run and added lightweight Code Quality section to main.md. No .golangci.yml config file - using golangci-lint's default configuration (5 standard linters). Linting runs before build in development (non-blocking) and in CI (blocking).

Copilot AI changed the title Fix CGO build flags in scripts/run to match CI configuration Add golangci-lint integration to local development workflow Dec 23, 2025
@wham
Copy link
Owner

wham commented Dec 23, 2025

@copilot When the lint is run via build.yml, there's no errors. But when I run it locally via golangci-lint run --timeout=5m, there's plenty of errors. Why?

Copy link
Contributor Author

Copilot AI commented Dec 23, 2025

@copilot When the lint is run via build.yml, there's no errors. But when I run it locally via golangci-lint run --timeout=5m, there's plenty of errors. Why?

The golangci-lint-action@v6 used in CI only reports new issues introduced in the PR by default (using --new-from-rev flag). All 11 issues found locally existed before this PR, so CI shows 0 issues. Running locally with golangci-lint run --timeout=5m --new-from-rev=5377241 (the base commit) also shows 0 issues. To see all issues in CI like you do locally, add only-new-issues: false to the action configuration in build.yml.

@wham
Copy link
Owner

wham commented Dec 23, 2025

@copilot I understand! That's good. Next please fix all the current issues.

…eck suggestions

Co-authored-by: wham <448809+wham@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 23, 2025

@copilot I understand! That's good. Next please fix all the current issues.

Fixed all 11 linting issues in commit f1f549f. Changes include: proper error checking for all Close() calls (8 errcheck issues), removed redundant type declaration, applied De Morgan's law, and fixed error message capitalization (3 staticcheck issues). The codebase now passes golangci-lint with 0 issues.

Copilot AI changed the title Add golangci-lint integration to local development workflow Add golangci-lint integration and fix all linting issues Dec 23, 2025
@wham wham marked this pull request as ready for review December 23, 2025 22:57
@wham wham added the minor label Dec 23, 2025
@wham wham merged commit 4ef3591 into main Dec 23, 2025
6 checks passed
@wham wham deleted the copilot/add-linter-to-go-code branch December 23, 2025 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants