Skip to content

Conversation

@Abirdcfly
Copy link
Contributor

Signed-off-by: Abirdcfly [email protected]

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #

What this PR does / why we need it:

@Abirdcfly Abirdcfly requested a review from aunjgr as a code owner August 10, 2022 17:13
@CLAassistant
Copy link

CLAassistant commented Aug 10, 2022

CLA assistant check
All committers have signed the CLA.

@matrix-meow matrix-meow added the size/XS Denotes a PR that changes [1, 9] lines label Aug 10, 2022
@mergify mergify bot added the kind/test-ci label Aug 10, 2022
@lni
Copy link
Contributor

lni commented Aug 10, 2022

@Abirdcfly your lint tool is pretty cool. why don't you just upstream it? ;)

@Abirdcfly
Copy link
Contributor Author

I use revive, but after recent practice, I found some false positives here (not errors reported as errors)

$ go install github.com/mgechev/revive@master

$ cat revive-config-unreach.toml
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0

[rule.unreachable-code]

$ revive -config revive-config-unreach.toml -exclude ./vendor/... ./...

@lni
Copy link
Contributor

lni commented Aug 11, 2022

I use revive, but after recent practice, I found some false positives here (not errors reported as errors)

$ go install github.com/mgechev/revive@master

$ cat revive-config-unreach.toml
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0

[rule.unreachable-code]

$ revive -config revive-config-unreach.toml -exclude ./vendor/... ./...

Thanks for the PR.

We briefly talked about revive within the team last week, seems to be a very decent SCA tool, haven't got chance to get it deployed. Care to send in a PR to help setting up revive for this project? That will be super useful.

We have all our static check stuff in the Makefile, the install-static-check-tools target installs required tools, you need to get a selected version of revive installed in that target. Actual checks is done in the static-check target. Configuration files for those static check tools are kept in the root of the repo.

Looking forward to your PR. ;)

@fengttt fengttt merged commit 9fde1fa into matrixorigin:main Aug 17, 2022
@Abirdcfly Abirdcfly deleted the patch-1 branch August 17, 2022 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/test-ci size/XS Denotes a PR that changes [1, 9] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants