Added golangci-lint with most linters disabled #598
Conversation
Signed-off-by: Arjun Naik <[email protected]>
arjunrn
changed the title
openshift-ci
bot
added
lgtm
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
6 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Makefile
Outdated
| $(GOLANGCI_LINT_BIN) run -c .golangci.yaml --deadline=30m | ||
|
|
||
| $(GOLANGCI_LINT_BIN): | ||
| curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b ${BIN_DIR} ${GOLANGCI_LINT_VERSION} |
There was a problem hiding this comment.
Installing from master seems dicy, since we could break as master becomes more strict. Especially once we fork off of this branch into long-lived, hopefully quiet release branches, it would be nice to have this pinned to a specific version which we explicitly bump periodically.
Also, downloading a script and running it directly is fairly risky. Can we use hashpipe or something similar to ensure we are only running the script we expect to get?
There was a problem hiding this comment.
I've moved the installer script into the hack directory. Note that the installer script takes a version parameter for binary it downloads. But like you pointed out curl-piping a bad idea. But having the installation script in repo should be good enough. WDYT?
There was a problem hiding this comment.
that's still pulling both the checksum and the binary from the same location. But either we can replace the script with something similar to pull our pinned version and compare with a local hash, or we can include a cache of the checksum file in our repo and twiddle the script to skip the checksum-fetching step so it uses our local copy.
There was a problem hiding this comment.
@wking I've changed the script so that the checksums are part of the script and the version is fixed. It only supports linux and darwin for now but should be enough for the time being.
There was a problem hiding this comment.
This looks good to me.
There was a problem hiding this comment.
Looks good to me to.
/hold cancel
Signed-off-by: Arjun Naik <[email protected]>
Signed-off-by: Arjun Naik <[email protected]>
|
/lgtm /hold for @wking for to take a look. |
openshift-ci
bot
added
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: arjunrn, LalatenduMohanty, vrutkovs The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/test e2e-agnostic |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
6 participants
Added a golangci-lint to code linting with most of the linters/analyzers disabled. Once this is merged I will enable this as a step in openshift/release and subsequently enable each of the linters individually to make the code reviews easier.