Skip to content

Conversation

@vertex451
Copy link
Contributor

@vertex451 vertex451 commented Jan 28, 2026

Description

Issues:

  • .golangci linter was badly formatted, some sections was ignored.
  • the --issues-exit-code=0 flag was used, which causes silent fails.
  • recommended linters were missing.

Changes

  • No logic changes
  • %w wraps errors instead of converting them to strings. This preserves the error chain for errors.Is() and errors.Unwrap().
  • Removed unused code

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

@coderabbitai
Copy link

coderabbitai bot commented Jan 28, 2026

Walkthrough

This PR updates Go linting configuration to enable additional linters and reorganize rules structure, removes unused SDK-specific type definitions from upgrade tests, and adds clarifying comments to escrow payment closure handling without altering functionality.

Changes

Cohort / File(s) Summary
Linting Configuration
.golangci.yaml, make/lint.mk
Enhanced linter coverage by enabling bodyclose, cyclop, errorlint, nilerr, nilnil, noctx, revive linters with increased issue limits; reorganized rule hierarchy with presets and settings sections; updated lint-go target to use golangci-lint run command with 10m timeout instead of prior invocation.
Test Cleanup
tests/upgrade/sdktypes.go, x/escrow/keeper/keeper_test.go, x/market/handler/handler_test.go
Removed entire SDK-specific type definitions file (nodeStatus, votingParams, depositParams, govParams, proposalResp, proposalsResp, SoftwareUpgradeProposal, ProposalMsg); deleted local kTestSuite type and createDeployment() test helper method.
Escrow Hook Documentation
upgrades/software/v1.1.0/upgrade.go, x/market/hooks/hooks.go
Added explanatory comments and nolint:nilerr annotation in OnEscrowPaymentClosed hook to document intentional silencing of non-lease payment closures without functional behavior changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • chainzero

Poem

🐰 Lint rules now flourish, configuration blooms,
Test clutter cleared from upgrade rooms,
Comments whisper why payments are ignored,
The code grows cleaner—consensus restored!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: linter issues' accurately describes the main purpose of the PR, which involves fixing linter configuration and resolving linter-related issues across multiple files.
Description check ✅ Passed The description is directly related to the changeset, explaining the rationale for configuration updates, removed flags, and code cleanup associated with linting improvements.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch artem/fix-linter-issues

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

- unconvert
- unparam
- unused
settings:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

that part has wrong indentation

.PHONY: lint-go
lint-go: $(GOLANGCI_LINT)
$(GOLANGCI_LINT_RUN) ./... --issues-exit-code=0 --timeout=10m
$(GOLANGCI_LINT) run ./... --timeout=10m
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lets fail in case of linter errors

@vertex451 vertex451 self-assigned this Jan 28, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @.golangci.yaml:
- Around line 42-45: The path exclusion patterns `third_party$`, `builtin$`, and
`examples$` are regexes anchored to the very end of the path so they won't match
files inside those directories (e.g., `third_party/somefile.go`); update those
patterns to match directory prefixes instead (for example use patterns that
start with the directory name and allow a trailing slash or any following path
segment such as `^third_party(/|$)`, `^builtin(/|$)`, `^examples(/|$)` or
`^third_party/.*`, `^builtin/.*`, `^examples/.*`) so that the `paths:` entries
will correctly exclude all files under those directories.

Comment on lines +42 to +45
paths:
- third_party$
- builtin$
- examples$
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, locate and examine the .golangci.yaml file
find . -name ".golangci.yaml" -o -name ".golangci.yml" | head -5

Repository: akash-network/node

Length of output: 78


🏁 Script executed:

# Check if the directories mentioned exist
fd -t d -name "third_party" -o -name "builtin" -o -name "examples" | head -10

Repository: akash-network/node

Length of output: 292


🌐 Web query:

golangci-lint path exclusion pattern syntax glob regex

💡 Result:

In golangci-lint, “path exclusion patterns” are usually regular expressions (regex), not glob—but a few linter-specific settings explicitly use glob.

Regex (most common)

run.skip-dirs, run.skip-files

These accept regexp patterns (matched against the path; commonly treated as “full path”). Example patterns like (^|/)vendor($|/) are used to skip directories. (github.com)

linters.exclusions.paths, linters.exclusions.rules[*].path, path-except

These are used to exclude reported issues by path, and the examples in the docs are clearly regex (e.g. (.+)_test\.go, _test\.go). (golangci-lint.run)

Relative path base (run.relative-path-mode)

What the regex is matched against depends on how golangci-lint computes “relative paths” (e.g. relative to go.mod, git root, config file, or working dir). (golangci-lint.run)

Glob (only where explicitly documented)

Some linter settings explicitly say they accept glob patterns. For example:

  • ruleguard.rules: “Glob patterns such as rules-*.go may be specified.” (golangci-lint.run)
  • protogetter.skip-files: “Skip files matching the specified glob pattern…” (examples include *.pb.go, */vendor/*). (golangci-lint.run)

Practical takeaway

  • If you’re configuring run skips or issue exclusions by path: write regex (RE2/Go-style), e.g. .*_test\.go$, (^|/)vendor($|/). (golangci-lint.run)
  • If you want glob, use only the options that explicitly document glob support (typically linter-specific settings). (golangci-lint.run)

Citations:


🏁 Script executed:

# Read the .golangci.yaml file to see the context around lines 42-45
cat -n .golangci.yaml | sed -n '30,50p'

Repository: akash-network/node

Length of output: 629


🏁 Script executed:

# Properly search for the directories mentioned
find . -type d \( -name "third_party" -o -name "builtin" -o -name "examples" \) 2>/dev/null

Repository: akash-network/node

Length of output: 44


🏁 Script executed:

# Check if there are any Go files in these directories (if they exist)
find . -type f -name "*.go" -path "*/third_party/*" -o -name "*.go" -path "*/builtin/*" -o -name "*.go" -path "*/examples/*" 2>/dev/null | head -20

Repository: akash-network/node

Length of output: 44


🏁 Script executed:

# Let's check the golangci-lint documentation more carefully
# Look for any comments or documentation in the repo about these patterns
grep -r "third_party\|builtin\|examples" .golangci.yaml

Repository: akash-network/node

Length of output: 173


🌐 Web query:

golangci-lint exclusions paths regex pattern matching behavior

💡 Result:

In golangci-lint, “exclusion paths” (e.g. linters.exclusions.paths, linters.exclusions.rules[].path, path-except) are treated as Go regular expressions (RE2) and are matched against the file path string (not glob patterns). (golangci-lint.run)

Key behavior:

  • What string is matched: the (relative) file path for the issue/file; patterns like (.+)_test\.go work because the matcher sees the path text. (golangci-lint.run)
  • Relative-to what? Controlled by run.relative-path-mode (used by exclusions, among other things). Options: gomod, gitroot, cfg, or wd (not recommended). Default is cfg in config v2. (golangci-lint.run)
  • Path separators: / in patterns is treated portably (documented as being replaced by the OS path separator) so patterns can work on Windows too. (golangci-lint.run)
  • Not implicitly anchored: regexes match like normal RE2—if you want “whole path” matching, add ^...$ yourself; if you want “directory only”, use an explicit boundary (e.g. (^|.*/)generated(/|$)).

Legacy note (v1 docs / older configs): exclusions were described as matching paths relative to the current working directory, and --path-prefix could affect matching. (golangci.github.io)

Citations:


🏁 Script executed:

# Check if there's a go.mod or any indication of project structure
head -20 go.mod 2>/dev/null || echo "No go.mod found in expected location"

Repository: akash-network/node

Length of output: 573


Path exclusion patterns use regex and won't exclude files within these directories.

The patterns third_party$, builtin$, examples$ are Go regular expressions (not glob). The $ anchor matches only the end of the path string. A path like third_party/somefile.go ends with .go, not third_party, so it won't be excluded.

If the intent is to exclude all files within these directories, adjust the patterns to:

🔧 Suggested fix
     paths:
-      - third_party$
-      - builtin$
-      - examples$
+      - third_party/
+      - builtin/
+      - examples/
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
paths:
- third_party$
- builtin$
- examples$
paths:
- third_party/
- builtin/
- examples/
🤖 Prompt for AI Agents
In @.golangci.yaml around lines 42 - 45, The path exclusion patterns
`third_party$`, `builtin$`, and `examples$` are regexes anchored to the very end
of the path so they won't match files inside those directories (e.g.,
`third_party/somefile.go`); update those patterns to match directory prefixes
instead (for example use patterns that start with the directory name and allow a
trailing slash or any following path segment such as `^third_party(/|$)`,
`^builtin(/|$)`, `^examples(/|$)` or `^third_party/.*`, `^builtin/.*`,
`^examples/.*`) so that the `paths:` entries will correctly exclude all files
under those directories.

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