Merged
Conversation
Reviewer's GuideUpdates the project to Go 1.26 across tooling and CI, refreshes golangci-lint to v2, and modernizes its configuration including linter selection and exclusion rules, while documenting the dependency change in the changelog. Flow diagram for updated golangci lint configurationflowchart TD
RunConfig[run
go 1_26
tests false]
LintersDefault[linters
default all]
DisabledLinters[disabled linters
containedctx
copyloopvar
cyclop
depguard
err113
exhaustive
exhaustruct
forcetypeassert
funlen
funcorder
gochecknoglobals
gocognit
gocritic
gocyclo
godoclint
godox
gomoddirectives
goprintffuncname
gosec
importas
lll
mnd
musttag
nestif
nilnil
noctx
noinlineerr
nlreturn
prealloc
recvcheck
revive
tagliatelle
unparam
wastedassign
wrapcheck
wsl]
LinterSettings[settings
lll line_length 120
varnamelen min_name_length 1]
Exclusions[exclusions
generated lax]
Presets[presets
comments
common_false_positives
legacy
std_error_handling]
Rules[rules
linter mnd excluded in _test\.go]
Paths[paths
third_party$
builtin$
examples$]
RunConfig --> LintersDefault
LintersDefault --> DisabledLinters
LintersDefault --> LinterSettings
LintersDefault --> Exclusions
Exclusions --> Presets
Exclusions --> Rules
Exclusions --> Paths
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- In the workflows you’re updating, consider also bumping
actions/setup-goandactions/checkoutto their latest major versions (e.g.,setup-go@v5,checkout@v4) to avoid relying on deprecated actions. - The new
.golangci.yamlexplicitly disables several safety/security-related linters (e.g.,gosec,noctx,noinlineerr); if that’s intentional, it may be worth narrowing those disables (e.g., viapaths/rules) rather than turning them off globally so you still catch issues in most of the codebase.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the workflows you’re updating, consider also bumping `actions/setup-go` and `actions/checkout` to their latest major versions (e.g., `setup-go@v5`, `checkout@v4`) to avoid relying on deprecated actions.
- The new `.golangci.yaml` explicitly disables several safety/security-related linters (e.g., `gosec`, `noctx`, `noinlineerr`); if that’s intentional, it may be worth narrowing those disables (e.g., via `paths`/`rules`) rather than turning them off globally so you still catch issues in most of the codebase.
## Individual Comments
### Comment 1
<location path=".golangci.yaml" line_range="6" />
<code_context>
-
linters:
- enable-all: true
+ default: all
disable:
- - typecheck
</code_context>
<issue_to_address>
**issue:** The `default: all` setting under `linters` may not be a recognized key in the v2 golangci-lint config schema.
In v2 configs, the supported way to control linters is via `enable`, `disable`, and `presets`; `default: all` isn’t part of the documented schema and may be ignored or fail validation. Please verify against the v2 reference and switch to the supported fields so the intended linters are reliably applied.
</issue_to_address>
### Comment 2
<location path=".golangci.yaml" line_range="44-49" />
<code_context>
+ - wastedassign
+ - wrapcheck
+ - wsl
+ settings:
+ lll:
+ line-length: 120
+ varnamelen:
+ min-name-length: 1
+ exclusions:
+ generated: lax
</code_context>
<issue_to_address>
**issue (bug_risk):** Linter-specific settings are nested under `linters.settings`, which may not match golangci-lint’s expected config layout.
These settings used to live under `linters-settings`, which aligns with the v1 docs. In v2, examples still show a top-level `linters-settings` rather than `linters: settings:`. If this nesting isn’t supported, `lll` and `varnamelen` configs may be silently ignored. Please verify against the v2 golangci-lint config docs and adjust the structure so these options are applied.
</issue_to_address>
### Comment 3
<location path=".github/workflows/release.yaml" line_range="15-18" />
<code_context>
uses: actions/setup-go@v1
with:
- go-version: 1.24.x
+ go-version: 1.26.x
- name: Run GoReleaser (dry run)
env:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using `actions/setup-go@v1` with a future Go version can be brittle and may not be supported.
Since `setup-go@v1` is outdated and has had breaking changes and deprecations, using it with Go 1.26 increases the chance of workflow failures. Please upgrade to a maintained major version (e.g., `actions/setup-go@v5`) that explicitly supports newer Go releases.
```suggestion
- name: Set up Go
uses: actions/setup-go@v5
with:
go-version: 1.26.x
```
</issue_to_address>
### Comment 4
<location path="docs/docs/changelog.md" line_range="8" />
<code_context>
## [Unreleased](https://github.com/lets-cli/lets/releases/tag/v0.0.X)
+* `[Dependency]` update go to `1.26`
* `[Added]` Show similar command suggestions on typos.
* `[Changed]` Exit code 2 on unknown command.
</code_context>
<issue_to_address>
**nitpick (typo):** Capitalize "Go" as the language name.
In this changelog entry, update `go` to `Go`: `*
* `[Dependency]` update Go to `1.26``.
```suggestion
* `[Dependency]` update Go to `1.26`
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| linters: | ||
| enable-all: true | ||
| default: all |
There was a problem hiding this comment.
issue: The default: all setting under linters may not be a recognized key in the v2 golangci-lint config schema.
In v2 configs, the supported way to control linters is via enable, disable, and presets; default: all isn’t part of the documented schema and may be ignored or fail validation. Please verify against the v2 reference and switch to the supported fields so the intended linters are reliably applied.
|
|
||
| ## [Unreleased](https://github.com/lets-cli/lets/releases/tag/v0.0.X) | ||
|
|
||
| * `[Dependency]` update go to `1.26` |
There was a problem hiding this comment.
nitpick (typo): Capitalize "Go" as the language name.
In this changelog entry, update go to Go: `*
[Dependency]update Go to `1.26``.
Suggested change
| * `[Dependency]` update go to `1.26` | |
| * `[Dependency]` update Go to `1.26` |
Contributor
Greptile SummaryThis PR is a straightforward Go toolchain upgrade from 1.24 → 1.26, consistently applied across Key observations:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
PR[PR: Bump Go to 1.26] --> GM[go.mod\ngo 1.26]
PR --> DF[Dockerfile\ngolang:1.26-bookworm\ngolangci-lint:v2.11.3-alpine]
PR --> GL[.golangci.yaml\nversion: 2 format\ngo: 1.26]
PR --> TW[test.yaml\nsetup-go: 1.26.x]
PR --> RW[release.yaml\nsetup-go: 1.26.x\nGOLANG_CROSS_VERSION: v1.26]
TW --> CI1[Unit tests\nmacOS + Ubuntu]
TW --> CI2[Bats integration tests]
TW --> CI3[Lint job\nlints lint]
RW --> REL[GoReleaser cross-compile\nghcr.io/goreleaser/goreleaser-cross:v1.26]
CI3 -- uses --> DF
CI3 -- uses --> GL
Last reviewed commit: e121c50 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| env: | ||
| PACKAGE_NAME: github.com/lets-cli/lets | ||
| GOLANG_CROSS_VERSION: v1.24 | ||
| GOLANG_CROSS_VERSION: v1.26 |
Contributor
There was a problem hiding this comment.
GOLANG_CROSS_VERSION is set to v1.26 (no patch component). The ghcr.io/goreleaser/goreleaser-cross images are typically published with full patch version tags (e.g., v1.26.0, v1.26.1). If the registry does not provide a floating v1.26 tag, both the dry-run and release steps will fail with an image-not-found error at runtime.
The same pattern existed previously (v1.24), so this may have been fine in practice — but it's worth pinning to a specific patch tag to make the workflow reproducible and avoid surprises when a new patch is published.
Suggested change
| GOLANG_CROSS_VERSION: v1.26 | |
| GOLANG_CROSS_VERSION: v1.26.0 |
The same change should also be applied on line 36.
| module github.com/lets-cli/lets | ||
|
|
||
| go 1.24 | ||
| go 1.26 |
Contributor
There was a problem hiding this comment.
toolchain directive
Since Go 1.21, the recommended practice is to pair the go directive with an explicit toolchain directive that pins the exact toolchain version. Without it, developers and CI with a newer toolchain installed may silently use a different patch than intended.
Suggested change
| go 1.26 | |
| go 1.26 | |
| toolchain go1.26.0 |
Comment on lines
7
to
+43
| disable: | ||
| - typecheck | ||
| - gomoddirectives | ||
| - containedctx | ||
| - gochecknoglobals | ||
| - goimports | ||
| - funlen | ||
| - godox | ||
| - maligned | ||
| - goerr113 | ||
| - exhaustivestruct | ||
| - wrapcheck | ||
| - prealloc # enable it sometimes | ||
| - wsl | ||
| - ifshort | ||
| - unparam | ||
| - copyloopvar | ||
| - cyclop | ||
| - gocyclo | ||
| - depguard | ||
| - err113 | ||
| - exhaustive | ||
| - exhaustruct | ||
| - forcetypeassert | ||
| - funlen | ||
| - funcorder | ||
| - gochecknoglobals | ||
| - gocognit | ||
| - tagliatelle | ||
| - nestif | ||
| - nlreturn | ||
| - gocritic | ||
| - gocyclo | ||
| - godoclint | ||
| - godox | ||
| - gomoddirectives | ||
| - goprintffuncname | ||
| - exhaustruct | ||
| - wastedassign | ||
| - gosec | ||
| - importas | ||
| - lll | ||
| - mnd | ||
| - musttag | ||
| - nestif | ||
| - nilnil | ||
| - noctx | ||
| - noinlineerr | ||
| - nlreturn | ||
| - prealloc | ||
| - recvcheck | ||
| - musttag | ||
| - mnd | ||
| - lll | ||
| - gocritic | ||
| - forcetypeassert | ||
| - exhaustive | ||
| - depguard | ||
| - revive | ||
| - gosec | ||
| - copyloopvar | ||
|
|
||
| linters-settings: | ||
| lll: | ||
| line-length: 120 | ||
| varnamelen: | ||
| min-name-length: 1 | ||
|
|
||
| issues: | ||
| exclude-rules: | ||
| - path: _test\.go | ||
| linters: | ||
| - gomnd | ||
| - path: set\.go | ||
| linters: | ||
| - typecheck | ||
| - tagliatelle | ||
| - unparam | ||
| - wastedassign | ||
| - wrapcheck | ||
| - wsl |
Contributor
There was a problem hiding this comment.
goimports no longer explicitly disabled
The previous config explicitly disabled goimports. The new config migrates to default: all (golangci-lint v2) but goimports is absent from the disable list. If goimports is included in the v2 default set, any file whose imports are not in goimports order will now produce lint failures and could break the lint CI job.
It's worth verifying whether the legacy exclusion preset covers this, or whether goimports should be added back to the disable list to preserve prior behaviour:
Suggested change
| disable: | |
| - typecheck | |
| - gomoddirectives | |
| - containedctx | |
| - gochecknoglobals | |
| - goimports | |
| - funlen | |
| - godox | |
| - maligned | |
| - goerr113 | |
| - exhaustivestruct | |
| - wrapcheck | |
| - prealloc # enable it sometimes | |
| - wsl | |
| - ifshort | |
| - unparam | |
| - copyloopvar | |
| - cyclop | |
| - gocyclo | |
| - depguard | |
| - err113 | |
| - exhaustive | |
| - exhaustruct | |
| - forcetypeassert | |
| - funlen | |
| - funcorder | |
| - gochecknoglobals | |
| - gocognit | |
| - tagliatelle | |
| - nestif | |
| - nlreturn | |
| - gocritic | |
| - gocyclo | |
| - godoclint | |
| - godox | |
| - gomoddirectives | |
| - goprintffuncname | |
| - exhaustruct | |
| - wastedassign | |
| - gosec | |
| - importas | |
| - lll | |
| - mnd | |
| - musttag | |
| - nestif | |
| - nilnil | |
| - noctx | |
| - noinlineerr | |
| - nlreturn | |
| - prealloc | |
| - recvcheck | |
| - musttag | |
| - mnd | |
| - lll | |
| - gocritic | |
| - forcetypeassert | |
| - exhaustive | |
| - depguard | |
| - revive | |
| - gosec | |
| - copyloopvar | |
| linters-settings: | |
| lll: | |
| line-length: 120 | |
| varnamelen: | |
| min-name-length: 1 | |
| issues: | |
| exclude-rules: | |
| - path: _test\.go | |
| linters: | |
| - gomnd | |
| - path: set\.go | |
| linters: | |
| - typecheck | |
| - tagliatelle | |
| - unparam | |
| - wastedassign | |
| - wrapcheck | |
| - wsl | |
| disable: | |
| - containedctx | |
| - copyloopvar | |
| - cyclop | |
| - depguard | |
| - err113 | |
| - exhaustive | |
| - exhaustruct | |
| - forcetypeassert | |
| - funlen | |
| - funcorder | |
| - gochecknoglobals | |
| - gocognit | |
| - gocritic | |
| - gocyclo | |
| - godoclint | |
| - godox | |
| - goimports | |
| - gomoddirectives | |
| - goprintffuncname | |
| - gosec | |
| - importas | |
| - lll | |
| - mnd | |
| - musttag | |
| - nestif | |
| - nilnil | |
| - noctx | |
| - noinlineerr | |
| - nlreturn | |
| - prealloc | |
| - recvcheck | |
| - revive | |
| - tagliatelle | |
| - unparam | |
| - wastedassign | |
| - wrapcheck | |
| - wsl |
kindermax
force-pushed
the
codex/update-lets-to-go-126
branch
from
e121c50 to
08e42dc
Compare
kindermax
force-pushed
the
codex/update-lets-to-go-126
branch
from
65e7282 to
a688482
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
Summary by Sourcery
Update the project to Go 1.26 and refresh linting configuration to match the newer toolchain.
Enhancements:
Build:
CI:
Documentation: