feat(golang): add golangci-lint 2.x support#53
Conversation
Add version detection to handle golangci-lint 2.x which has different default linters and output format flags. Maintain backward compatibility with 1.x versions.
turo-ai-code-review
bot
added
the
claude-in-review
![turo-ai-code-review[bot]](https://avatars.githubusercontent.com/in/2040854?s=60&v=4){kind=small}
| else | ||
| # version < 1.55.0 → use --out-format | ||
| format_flag="--out-format" | ||
| fi | ||
| ## golangci-lint 1.x |
There was a problem hiding this comment.
suggestion (consistency): This uses a different extraction pattern (grep -oP) than the major version extraction (line 11 uses sed -n). Consider using a single consistent approach for maintainability.
Why this matters: Using different regex engines (PCRE via grep -P vs sed) makes the code harder to maintain and reason about. If one pattern needs updating, it's unclear whether both should be updated.
Suggested approach: Extract the full version once at the top, then derive both major and full version from it:
| else | |
| # version < 1.55.0 → use --out-format | |
| format_flag="--out-format" | |
| fi | |
| ## golangci-lint 1.x | |
| ## Extract full semver for 1.x format flag detection | |
| version=$(echo "$version_output" | sed -n 's/.*version \([0-9]*\.[0-9]*\.[0-9]*\).*/\1/p') |
This maintains consistency with the sed-based approach while supporting the 1.55.0 comparison logic.
| version=$(echo "$version_output" | grep -oP 'version \K[0-9]+\.[0-9]+\.[0-9]+') | ||
| ## Extract major version number (e.g., 1 or 2) | ||
| major_version=$(echo "$version_output" | sed -n 's/.*version \([0-9]*\).*/\1/p') | ||
|
|
There was a problem hiding this comment.
issue (reliability): The sed pattern 's/.*version \([0-9]*\).*/\1/p' may silently fail if golangci-lint --version output format changes or doesn't match expectations.
Why this matters: If the extraction fails, $major_version will be empty, causing the comparison [ "$major_version" = "2" ] to always fail. This would silently fall back to 1.x behavior even when running 2.x, potentially causing linter failures or unexpected behavior.
Suggested fix: Add explicit validation after extraction:
| major_version=$(echo "$version_output" | sed -n 's/.*version \([0-9]*\).*/\1/p') | |
| ## Validate major version was successfully extracted | |
| if [ -z "$major_version" ]; then | |
| echo "Error: Failed to extract golangci-lint major version from: $version_output" >&2 | |
| return 1 | |
| fi |
This ensures the script fails fast with a clear error message rather than silently misbehaving.
Overall Assessment: APPROVEThis PR adds support for golangci-lint 2.x while maintaining backward compatibility with 1.x versions. The implementation correctly identifies that 2.x has different default linters and removes deprecated format flags. The approach is sound and the logic properly branches based on detected major version. Critical Issues FoundNone - the logic is functionally correct and will work as intended. Performance ConcernsNone identified. Business Logic Review RequiredNone - this is a straightforward tooling compatibility update. Notes for Human ReviewersTwo inline comments have been posted suggesting reliability improvements:
These are code quality suggestions, not blocking issues. The current implementation will work correctly when golangci-lint version output follows expected format, which is the standard case. |
There was a problem hiding this comment.
Automated review complete. No critical issues found.
See this FAQ on how to interact with this review bot.
turo-ai-code-review
bot
removed
the
claude-in-review
2 participants
Description
Add version detection to handle golangci-lint 2.x which has different default linters and output format flags. Maintain backward compatibility with 1.x versions.
Not as part of this, but I wonder if we should be moving to https://github.com/golangci/golangci-lint/blob/main/.pre-commit-hooks.yaml. They have their own pre-commit hooks now.
Changes
🚀 PR created with fotingo