vet: enforce revive linter #7589
Conversation
purnesh42H
force-pushed
the
skip-revive-empty-block
branch
from
a7bf0f6 to
117d767
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7589 +/- ##
==========================================
- Coverage 81.94% 81.86% -0.08%
==========================================
Files 361 361
Lines 27813 27813
==========================================
- Hits 22790 22768 -22
- Misses 3832 3851 +19
- Partials 1191 1194 +3 |
scripts/revive.toml
Outdated
| # Disabled rules | ||
| [rule.empty-block] | ||
| Disabled = true |
There was a problem hiding this comment.
Since we have enough effective usages of empty-block, imo we should not bother about it at all.
| # Disabled rules | |
| [rule.empty-block] | |
| Disabled = true |
There was a problem hiding this comment.
Instead can we add explicit disable for the fixes that are in flight. That way we can rebase this change, and remove that check as we fix them. Seems cleaner to me. That way we can avoid having to see all the linter error annotations on PRs
wdyt?
There was a problem hiding this comment.
I like the idea. i modified to disable the rules that are yet to merged. Let me know if that looks good
scripts/vet.sh
Outdated
| @@ -178,7 +178,7 @@ done | |||
|
|
|||
| # Collection of revive linter analysis checks | |||
| REV_OUT="$(mktemp)" | |||
| revive -set_exit_status=1 -exclude "reflection/test/grpc_testing_not_regenerate/" -formatter plain ./... >"${REV_OUT}" || true | |||
| revive -set_exit_status=1 -exclude "reflection/test/grpc_testing_not_regenerate/" -formatter plain -config "$(dirname "$0")/revive.toml" ./... >"${REV_OUT}" || true | |||
There was a problem hiding this comment.
I think we can safely combine lines 180-184 to be something like:
| revive -set_exit_status=1 -exclude "reflection/test/grpc_testing_not_regenerate/" -formatter plain -config "$(dirname "$0")/revive.toml" ./... >"${REV_OUT}" || true | |
| revive \ | |
| -set_exit_status=1 \ | |
| -exclude "reflection/test/grpc_testing_not_regenerate/" \ | |
| -exclude "**/*.pb.go" \ | |
| -config "$(dirname "$0")/revive.toml" \ | |
| ./... |
purnesh42H
changed the title
There was a problem hiding this comment.
Let's revert these changes from this pr and add it to #7575
purnesh42H
force-pushed
the
skip-revive-empty-block
branch
from
3f90caf to
dad9ef0
Compare
rpc_util.go
Outdated
There was a problem hiding this comment.
I missed to comment on this file in the last pass
purnesh42H
force-pushed
the
skip-revive-empty-block
branch
from
dad9ef0 to
ba338c1
Compare
scripts/revive.toml
Outdated
| [rule.exported] | ||
| Disabled = true # TODO: Enable after existing issues are fixed | ||
| [rule.redefines-builtin-id] | ||
| Disabled = true # TODO: Enable after existing issues are fixed |
purnesh42H
force-pushed
the
skip-revive-empty-block
branch
from
88f4e34 to
9f7d269
Compare
purnesh42H
added
Type: Meta
Addresses: #7444
Enforce the revive linter with a configuration that enables only the rules that are already fixed, while keeping the rules that are yet to be fixed disabled. We will enable the remaining rules as we address and resolve them.
Recommended lint rules by revive (cover default golint rules) except empty-block
RELEASE NOTES: None