-
Notifications
You must be signed in to change notification settings - Fork 11
HYPERFLEET-376: Add linting and static analysis standard #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
HYPERFLEET-376: Add linting and static analysis standard #59
Conversation
- Add linting.md documenting the shared linting baseline for all HyperFleet Go repositories - Add reference golangci.yml configuration file - Document enabled linters with rationale (errcheck, govet, staticcheck, gosec, revive, gocritic, etc.) - Define standard exclusions for generated code and test files - Specify allowed and disallowed repository-specific overrides - Include CI integration guidelines (make lint target) Follow-up ticket created: HYPERFLEET-422 (Claude plugin integration)
WalkthroughThis PR introduces standardized linting configuration and documentation for HyperFleet Go repositories. It adds a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Pre-merge checks✅ Passed checks (3 passed)
Comment |
There was a problem hiding this 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
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hyperfleet/docs/linting.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
hyperfleet/docs/linting.md
188-188: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (2)
hyperfleet/docs/linting.md (2)
1-217: Good policy foundation and clear adoption guidance.The document establishes a well-structured linting baseline with:
- Clear articulation of enabled linters and their rationales (lines 13–46)
- Concrete configuration examples for each linter (lines 47–147)
- Effective guardrails on baseline enforcement (lines 159–163: disabling baseline linters is prohibited)
- Practical adoption steps (lines 209–217)
- Helpful distinction between allowed overrides (additional exclusions, new linters) and forbidden changes
The policy balances standardization with repository autonomy, which should facilitate adoption across HyperFleet repositories.
11-11: All referenced files exist and cross-file references are correct. The filesgolangci.yml,generated-code-policy.md, andmakefile-conventions.mdare present inhyperfleet/docs/with properly formatted relative links at lines 11, 106, 183, and 213. The golangci.yml file has valid structure withversion: "2".
| ```makefile | ||
| .PHONY: lint | ||
| lint: | ||
| golangci-lint run ./... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix hard tab in makefile code block.
Line 188 uses a hard tab (flagged by markdownlint-cli2 MD010). Replace with spaces for consistency.
🔎 Proposed fix
.PHONY: lint
lint:
- golangci-lint run ./...
+ golangci-lint run ./...Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
188-188: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🤖 Prompt for AI Agents
In hyperfleet/docs/linting.md around line 188, the code block line currently
contains a hard tab character (triggering markdownlint-cli2 MD010); replace the
hard tab with equivalent spaces (e.g., two or four spaces consistent with the
repo style) so the line reads "golangci-lint run ./..." using spaces only, and
ensure no other tabs remain in that fenced code block.
There was a problem hiding this 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
♻️ Duplicate comments (1)
hyperfleet/standards/linting.md (1)
188-188: Hard tab flagged in previous review.A past review identified that line 188 contains a hard tab character (markdownlint MD010). Replace the tab with spaces for consistency.
🧹 Nitpick comments (1)
hyperfleet/standards/golangci.yml (1)
80-95: Consider consistent path patterns.The
exclude-dirsat line 81 usespkg/api/openapiwithout a trailing slash, while theexclude-rulesat line 93 usespkg/api/openapi/with a trailing slash. Both should work, but for consistency, consider using the same pattern format throughout.🔎 Suggested consistency improvement
# Exclude generated code (backup pattern if exclude-dirs doesn't catch all) - - path: pkg/api/openapi/ + - path: pkg/api/openapi linters: - all
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hyperfleet/standards/golangci.ymlhyperfleet/standards/linting.md
🔇 Additional comments (6)
hyperfleet/standards/golangci.yml (3)
55-77: LGTM: Comprehensive linter configuration.The enabled linters match the documented baseline and provide good coverage across code quality, style, and security concerns.
99-99: Consider the implications ofnew: false.Setting
new: falsemeans golangci-lint will report all issues, not just new ones introduced in the current changes. While this is appropriate for initial adoption and getting a full baseline, it may produce overwhelming output in CI for repositories with existing violations. The documentation mentions creating a separate tracking ticket for fixing existing violations (line 216), which aligns with this setting.Consider whether this should remain
falselong-term or be changed totrueafter the baseline is established.
101-105: LGTM: Clear output configuration.The output format configuration provides helpful, readable output for developers with colored formatting and clear linter attribution.
hyperfleet/standards/linting.md (3)
106-106: The cross-reference togenerated-code-policy.mdis correct and the document exists athyperfleet/docs/generated-code-policy.md.
183-183: Cross-reference is valid. The filehyperfleet/docs/makefile-conventions.mdexists at the expected location and the reference in line 183 is correct.
204-208: No changes required. The specification of golangci-lint v2.x and theversion: 2configuration format is current and correct as of December 2025 (latest release v2.7.2).
| ## Linter Settings | ||
|
|
||
| ### errcheck | ||
|
|
||
| ```yaml | ||
| errcheck: | ||
| check-type-assertions: true # Check type assertion results | ||
| check-blank: true # Check assignments to blank identifier | ||
| ``` | ||
|
|
||
| ### govet | ||
|
|
||
| ```yaml | ||
| govet: | ||
| enable-all: true # Enable all govet checks | ||
| ``` | ||
|
|
||
| ### goconst | ||
|
|
||
| ```yaml | ||
| goconst: | ||
| min-len: 3 # Minimum string length | ||
| min-occurrences: 3 # Minimum occurrences before suggesting const | ||
| ``` | ||
|
|
||
| ### misspell | ||
|
|
||
| ```yaml | ||
| misspell: | ||
| locale: US # Use US English spelling | ||
| ``` | ||
|
|
||
| ### lll | ||
|
|
||
| ```yaml | ||
| lll: | ||
| line-length: 120 # Maximum line length | ||
| ``` | ||
|
|
||
| ### revive | ||
|
|
||
| ```yaml | ||
| revive: | ||
| rules: | ||
| - name: exported | ||
| severity: warning | ||
| disabled: true # Can be too noisy for internal packages | ||
| - name: unexported-return | ||
| severity: warning | ||
| disabled: false | ||
| - name: var-naming | ||
| severity: warning | ||
| disabled: false | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Document missing linter settings.
The golangci.yml configuration file includes settings for gofmt.simplify, unused.check-exported, and unparam.check-exported that are not documented here. For completeness, add sections documenting these settings.
🔎 Suggested additions
Add after the gofmt section around line 85:
### gofmt
```yaml
gofmt:
simplify: true # Apply code simplifications
Add after the `revive` section around line 100:
```markdown
### unused
```yaml
unused:
check-exported: false # Don't flag unused exported identifiers
unparam
unparam:
check-exported: false # Don't flag unused params in exported functions
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
hyperfleet/standards/linting.md around lines 47-101: the document omits three
linter settings present in golangci.yml; add a new "gofmt" section after the
existing gofmt heading (around line 85) documenting that gofmt.simplify is
enabled (i.e., simplify: true), and add two new sections after the revive block
(around line 100) named "unused" and "unparam" documenting that their
check-exported options are set to false (i.e., check-exported: false); ensure
each section includes a brief YAML snippet and a one-line comment explaining the
purpose of the setting.
</details>
<!-- fingerprinting:phantom:poseidon:puma -->
<!-- This is an auto-generated comment by CodeRabbit -->
| tests: true | ||
| modules-download-mode: readonly | ||
|
|
||
| linters-settings: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend adding exhaustive specifically for our enum types. As we add more cloud providers or cluster states, this linter will act as a safety net to ensure every part of the codebase is updated to handle those new types.
Summary
Adds a standardized linting and static analysis baseline for all HyperFleet Go repositories.
Changes
Enabled Linters
Key Decisions
exclude-dirsFollow-up
Test Plan
Closes HYPERFLEET-376
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.