Add staticcheck to CI #29492
Add staticcheck to CI #29492
Conversation
jbardin
force-pushed
the
jbardin/staticcheck
branch
from
d37208e to
1ae87f5
Compare
scripts/staticcheck.sh
Outdated
| echo "==> Checking that code complies with static analysis requirements..." | ||
| # The legacy code is frozen, and should not be updated. It will be removed once | ||
| # we can refactor the remote backends to no longer require it. | ||
| skip="internal/legacy|backend/remote-state/|internal/planproto|internal/tfplugin" |
There was a problem hiding this comment.
The above comment reads as if internal/planproto and internal/tfplugin fit in the category of "legacy code that will be removed", but I assume that isn't what you intended. To minimize confusion for future contributors, can we say something about excluding generated code in that comment too?
There was a problem hiding this comment.
thanks, that was updated from the old commit, but the comment is out of date. I'll re-word
There was a problem hiding this comment.
It might also be clearer to write out tfplugin5 and tfplugin6 separately here so that we can see exactly which packages are being excluded. 🤔
jbardin
force-pushed
the
jbardin/staticcheck
branch
from
1ae87f5 to
8e6860c
Compare
cleanup the old fmtcheck script while we're in here
jbardin
force-pushed
the
jbardin/staticcheck
branch
from
8e6860c to
863963e
Compare
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
2 participants
Add
staticcheck -checks 'all,-ST*'to the CI config.This tool was used extensively during refactoring, and has proven to be quite useful for catching common mistakes.
While we skip the stylistic checks here for now, we can work towards those too (or at least a subset of those, since we intentionally break some conventions like error string format).