Skip to content

Conversation

@JosiahWitt
Copy link
Owner

@JosiahWitt JosiahWitt commented Oct 25, 2025

PR Type

Enhancement, Tests


Description

  • Upgrade GitHub Actions to latest versions (setup-go@v6, checkout@v5, codecov-action@v5)

  • Expand Go version matrix from 1.14-1.19 to 1.14-1.25 for comprehensive testing

  • Add staticcheck linter job for Go 1.23+ with version 2025.1.1

  • Upgrade golangci-lint configuration to v2 format with updated linter settings

  • Fix unused parameter warnings by replacing named parameters with underscore

  • Update linter nolint comment from deprecated goerr113 to err113

  • Add Copilot instructions documentation for project architecture and patterns


Diagram Walkthrough

flowchart LR
  A["GitHub Actions<br/>Upgrade"] --> B["Test & Lint<br/>Jobs"]
  C["Go Version<br/>Matrix"] --> B
  D["Staticcheck<br/>Job"] --> B
  E["Golangci-lint<br/>Config v2"] --> B
  F["Code Cleanup<br/>Unused Params"] --> G["Linter<br/>Compliance"]
  H["Copilot<br/>Instructions"] --> I["Documentation"]
Loading

File Walkthrough

Relevant files
Configuration changes
ci.yml
Upgrade CI workflows and expand Go version coverage           

.github/workflows/ci.yml

  • Upgrade GitHub Actions: setup-go@v2→v6, checkout@v1→v5,
    codecov-action@v1→v5
  • Expand Go version matrix from 6 versions (1.14-1.19) to 12 versions
    (1.14-1.25)
  • Add fail-fast: false to test and lint jobs for parallel execution
  • Replace manual golangci-lint installation with
    golangci/golangci-lint-action@v8
  • Add new staticcheck job for Go 1.23+ using
    dominikh/staticcheck-action@v1
+64/-13 
.golangci.yml
Migrate golangci-lint config to v2 format                               

.golangci.yml

  • Upgrade configuration format from v1 to v2 with new structure
  • Change linters from enable-all to default: all with explicit disables
  • Remove deprecated linters (scopelint, varcheck, ifshort, interfacer,
    etc.)
  • Add new disabled linters (depguard, embeddedstructfieldcheck, wsl_v5,
    inamedparam)
  • Reorganize settings with exclusions presets and formatters
    configuration
  • Update nolint comment from goerr113 to err113 in exclude rules
+37/-28 
Tests
erkjson_test.go
Fix unused parameter warnings in test file                             

erkjson/erkjson_test.go

  • Replace unused parameter entry with underscore in 8 TypeCheck function
    signatures
  • Replace unused parameter kind with underscore in 4 KindStringFor
    method signatures
  • Reorder NewKindAsStringPtr function to appear before its usage in
    KindAsString type
+12/-12 
kind_test.go
Fix unused parameter and improve code organization             

kind_test.go

  • Replace unused parameter k with underscore in KindStringFor method
    signature
  • Reorder NewKindAsStringPtr function to appear before KindAsString
    methods
+6/-6     
Bug fix
error_export.go
Update deprecated linter comment                                                 

error_export.go

  • Update nolint comment from deprecated goerr113 to err113
+1/-1     
kind.go
Fix unused parameter warning                                                         

kind.go

  • Replace unused parameter kind with underscore in TemplateFuncsFor
    method signature
+1/-1     
Documentation
copilot-instructions.md
Add Copilot instructions for project architecture               

.github/copilot-instructions.md

  • Add comprehensive architecture overview of Erk error library
    components
  • Document error definition patterns and message template syntax
  • Provide error groups (erg) usage examples and best practices
  • Explain strict mode behavior and testing patterns with ensure library
  • Include development workflow, key interfaces, and common patterns
+130/-0 

@qodo-code-review
Copy link

Auto-approved PR

@qodo-code-review
Copy link

qodo-code-review bot commented Oct 25, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

qodo-code-review bot commented Oct 25, 2025

PR Code Suggestions ✨

  • Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

Previous suggestions

Suggestions up to commit a78255e
CategorySuggestion                                                                                                                                    Impact
Possible issue
Correct invalid linter version

Correct the golangci-lint version in the CI workflow from the invalid v2.5.0 to
a valid version like v1.59.1 to prevent the lint job from failing.

.github/workflows/ci.yml [74-77]

 - name: Lint
   uses: golangci/golangci-lint-action@v8
   with:
-    version: v2.5.0
+    version: v1.59.1
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that golangci-lint version v2.5.0 is invalid and will cause the CI lint job to fail, proposing a valid version to fix the issue.

High
High-level
Simplify the extensive CI matrix

Reduce the Go version matrix in the CI pipeline for both testing and linting. It
is recommended to test on a smaller, strategic set of versions (oldest, latest,
edge) and run the linter on just one recent version to improve CI efficiency and
reduce costs.

Examples:

.github/workflows/ci.yml [9-24]
      matrix:
        go-version:
          [
            "1.14",
            "1.15",
            "1.16",
            "1.17",
            "1.18",
            "1.19",
            "1.20",

 ... (clipped 6 lines)
.github/workflows/ci.yml [48-63]
      matrix:
        go-version:
          [
            "1.14",
            "1.15",
            "1.16",
            "1.17",
            "1.18",
            "1.19",
            "1.20",

 ... (clipped 6 lines)

Solution Walkthrough:

Before:

# .github/workflows/ci.yml
jobs:
  test:
    strategy:
      matrix:
        go-version: ["1.14", "1.15", ..., "1.25"] # 12 versions
    steps:
      - name: Test
        run: go test ./...

  lint:
    strategy:
      matrix:
        go-version: ["1.14", "1.15", ..., "1.25"] # 12 versions
    steps:
      - name: Lint
        uses: golangci/golangci-lint-action@v8

After:

# .github/workflows/ci.yml
jobs:
  test:
    strategy:
      matrix:
        # Test oldest supported, latest stable, and edge
        go-version: ["1.14", "1.24", "1.25"]
    steps:
      - name: Test
        run: go test ./...

  lint:
    # No matrix, run only on one recent version
    steps:
      - name: Set up Go
        uses: actions/setup-go@v6
        with:
          go-version: "1.25"
      - name: Lint
        uses: golangci/golangci-lint-action@v8
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a significant inefficiency in the CI configuration, where testing and linting on 12 Go versions is excessive and costly, proposing a much more practical and standard industry practice.

Medium
General
Expand Go versions for staticcheck

Expand the go-version matrix for the staticcheck job to include Go 1.22 and
update the misleading comment, as staticcheck supports more versions than just
1.23+.

.github/workflows/ci.yml [82-86]

 strategy:
   fail-fast: false
   matrix:
-    # Staticcheck requires Go 1.23+
-    go-version: ["1.23", "1.24", "1.25"]
+    # Staticcheck supports the last two major Go releases
+    go-version: ["1.22", "1.23", "1.24", "1.25"]
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly points out that staticcheck supports more Go versions than listed and proposes expanding the test matrix, which improves test coverage.

Low

@codecov
Copy link

codecov bot commented Oct 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (588cc97) to head (931c05d).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##            master       #48    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           13        13            
  Lines          322       428   +106     
==========================================
+ Hits           322       428   +106     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@qodo-code-review
Copy link

Persistent suggestions updated to latest commit 931c05d

@JosiahWitt JosiahWitt merged commit 5a04a40 into master Oct 25, 2025
29 checks passed
@JosiahWitt JosiahWitt deleted the go1.25 branch October 25, 2025 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants