valkey-io/valkey-go

GH action on PR and tests

Closed this issue · 2 comments

As far as I see the current GH actions are configured for only push action.
It would be great to be able to see the test is passing and maybe add some linting to the PR, such that we can increase code quality.

For example we use vet and staticcheck
https://github.com/zalando/skipper/blob/master/.github/workflows/pr.yaml
Of course we have to disable some checks because we can not just break lib users for the sake of a linter https://github.com/zalando/skipper/blob/master/Makefile#L129-L138

For security checks I can recommend https://github.com/zalando/skipper/blob/master/Makefile#L150-L160 , gosec I can not recommend, because of an annoying false positive rate, but govulncheck, capslock and osv-scanner are really great.

Hi @szuecs,

I have re-enabled GH actions on pull requests 3eb1b2c.

I found that vet and staticcheck with default rules and excluding tests, by grep -v not -tests=false, are good:

go vet ./...
staticcheck  ./... | (grep -v "_test.go:" && exit 1 || exit 0)
image

Their results are actionable, with no false positives and no unnecessary alerts.

Would you be willing to add them to the current https://github.com/valkey-io/valkey-go/blob/main/dockertest.sh?

On the other hand, the results of govulncheck, capslock and osv-scanner are quite confusing to me. They seem to be most related to dependencies including Go itself. That makes me feel we don't need to add them to RP checks since we already have Dependabot configured.

@rueian I disabled some checks and added comments on why. In the future we can fix some of them and others we will have to accept or add opt-out comments to API breaking change proposals that we do not want to do. One tiny code change to fix a linter finding is hopefully fine for this.