Add godoc linter

[dfawley]

Since removing golint, we've had numerous occasions of missing docstrings. This seems like the consensus replacement for golint: https://github.com/mgechev/revive -- we should set it up to at least check the docstrings.

EDIT:

• add the Revive inter in quite mode: tools: Add github.com/mgechev/revive #7472**

• Enforce Revive Linter: vet: enforce revive linter #7589

• fix below existing lint issues and enable the disabled rule

#7575 fixes

• 83 exported

#7574 fixes

• 8 package-comments

#7552 fixes

• 15 redefines-builtin-id

#7528 fixes

• 6 var-declaration
• 3 indent-error-flow
• 1 increment-decrement
• 1 superfluous-else

#7577 #7580 fixes

• 546 unused-parameter
• 28 var-naming (Ignore reflection/test/grpc_testing_not_regenerate/)
• 1 context-as-argument (Ignore reflection/test/grpc_testing_not_regenerate/)

[purnesh42H]

Before we enforce revive, we need to fix existing lint issues caught in #7472 as part of vet check.

Its fine to change several files in a single PR if they all fix one type of issue or are otherwise related. However, be reasonable and try to keep the PRs small enough to make it not take a long time to review and avoid merge conflicts.

[janardhanvissa]

@purnesh42H Please assign me this issue.

[janardhanvissa]

@purnesh42H Right now, In the tests (vet) I'm getting like don't use an underscore in package name https://revive.run/r#var-naming.
It's not expecting underscore but by default as it's generating from the proto file like ("google.golang.org/grpc/reflection/test/grpc_testing_not_regenerate"). So, what need to do for further. Please suggest.

[janardhanvissa]

@purnesh42H Fixed some of the godoc-linter issues. Please review and let me know if any changes are required. Please find the PR below.
#7501

[janardhanvissa]

@purnesh42H Fixed linter issues for superfluous-else, increment-decrement, indent-error-flow, var-declaration. Please review the changes.

janardhanvissa#1

[janardhanvissa]

@purnesh42H Fixed below godoc linter issues. Please find the PRs below. Please let me know if any changes are required.

15 redefines-builtin-id
8 package-comments
8 empty-block
6 var-declaration
3 indent-error-flow
2 increment-decrement
1 superfluous-else

janardhanvissa#1
janardhanvissa#2
janardhanvissa#3

[purnesh42H]

@purnesh42H Fixed linter issues for superfluous-else, increment-decrement, indent-error-flow, var-declaration. Please review the changes.

janardhanvissa#1

Thanks. LGTM. Please open the. PR in main repo.

[janardhanvissa]

@purnesh42H Fixed below godoc linter issues and raised a PR to main repo.

6 var-declaration
3 indent-error-flow
2 increment-decrement
1 superfluous-else

[janardhanvissa]

@purnesh42H Fixed below godoc revive linter issues and raised a PR to main repo. Please find the PR below.

#7532

• 15 redefines-builtin-id
• 8 package-comments
• 8 empty-block

[janardhanvissa]

@purnesh42H Fixed below godoc revive linter issues and raised a PR to main repo. Please find the PR below.

#7550

• 8 package-comments

[janardhanvissa]

@purnesh42H Fixed below godoc revive linter issues and raised a PR to main repo. Please find the PR below.

#7552

• 8 empty-block
• 15 redefines-builtin-id

[janardhanvissa]

@purnesh42H Facing issue in context-as-argument(context.Context) should be the first parameter. Please have a look at the readme file (reflection/test/grpc_testing_not_regenerate/README.md), as the testv3.go is generated from the older version of v3.
So, once we are updating and tried to generate the proto and this testv3 we are facing undefined function or var in other dependencies.
It points to some external dependency with testv3 so any change in testv3 will have an impact which is not in our scope. Because it's imported from another repo.
Suggest what we need to do to resolve this issue.
If we are going to regenerate the testv3.proto file we need to delete the testv3.go and then regenerate. Then it will impact other dependency files like dynamic.go

[janardhanvissa]

@purnesh42H Raised a separate PRs in forked branch. Please review the changes. Please find the PRs below.

janardhanvissa#6
janardhanvissa#7

[arvindbr8]

#7577 fixes:

546 unused-parameter (ignored)

and renamed the check again

cc: @purnesh42H

[purnesh42H]

@janardhanvissa can you do these since we include unused-parameter now

gcp/observability/logging_test.go:592:20: parameter 'ctx' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:592:41: parameter 'in' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:653:28: parameter 'ctx' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:653:49: parameter 'config' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:676:20: parameter 'ctx' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:676:41: parameter 'in' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:790:28: parameter 'ctx' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:790:49: parameter 'config' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:825:20: parameter 'ctx' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:825:41: parameter 'in' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:828:20: parameter 'ctx' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:828:41: parameter 'in' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:1128:28: parameter 'ctx' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:1128:49: parameter 'config' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:1152:20: parameter 'ctx' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:1152:41: parameter 'in' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:1252:28: parameter 'ctx' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter
gcp/observability/logging_test.go:1252:49: parameter 'config' seems to be unused, consider removing or renaming it as _ https://revive.run/r#unused-parameter

[janardhanvissa]

@purnesh42H Fixed above mentioned unused parameter linter issues and pushed the changes.

[arvindbr8]

Adding this here for context:

#7552 (comment)

Based on the discussion, we have sufficient cases where having an empty block is justifiable, so it's reasonable to include an "empty-block" check within the revive tool.