-
Notifications
You must be signed in to change notification settings - Fork 126
[chain] Apply exported
lint rule
#2018
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
.golangci.yml
Outdated
exclude: | ||
- "**/abi/**/*.go" | ||
- "**/api/**/*.go" | ||
- "**/auth/**/*.go" | ||
- "**/chainindexer/**/*.go" | ||
- "**/cli/**/*.go" | ||
- "**/cmd/**/*.go" | ||
- "**/codec/**/*.go" | ||
- "**/consts/**/*.go" | ||
- "**/context/**/*.go" | ||
- "**/crypto/**/*.go" | ||
- "**/event/**/*.go" | ||
- "**/examples/**/*.go" | ||
- "**/extension/**/*.go" | ||
- "**/fees/**/*.go" | ||
- "**/genesis/**/*.go" | ||
- "**/internal/**/*.go" | ||
- "**/keys/**/*.go" | ||
- "**/proto/**/*.go" | ||
- "**/pubsub/**/*.go" | ||
- "**/requester/**/*.go" | ||
- "**/snow/**/*.go" | ||
- "**/state/**/*.go" | ||
- "**/statesync/**/*.go" | ||
- "**/storage/**/*.go" | ||
- "**/tests/**/*.go" | ||
- "**/throughput/**/*.go" | ||
- "**/utils/**/*.go" | ||
- "**/vm/**/*.go" | ||
- "**/x/**/*.go" | ||
- "TEST" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally, a regex that includes all packages except chain
would've been better here. However, since we plan on applying the exported
rule to all public-facing packages, it might be better to leave it in this format.
export
lint ruleexported
lint rule
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why isn't exported
rule picking up on this... Will play a bit and see what I find out.
Len(context.Context) int // items | ||
Size(context.Context) int // bytes | ||
// Number of items | ||
Len(context.Context) int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linter doesn't seem to apply exported
rule correctly.
The Go documentation guidelines specify that documentation comments for exported identifiers should start with the name being declared. This helps with generated documentation, maintains a consistent style, and makes it clear what's being described, especially when multiple declarations are being documented in sequence.
// This lacks a comment
func HandleRequest(w http.ResponseWriter, r *http.Request) {
// ...
}
// Processes incoming HTTP requests
func HandleRequest(w http.ResponseWriter, r *http.Request) {
// ...
}
Both of these functions should be targeted.
GoLand IDE is reporting invalid comment pattern as well.
Putting this back into draft; realized that |
Co-authored-by: Elvis <[email protected]> Signed-off-by: rodrigo <[email protected]>
.golangci.yml
Outdated
exclude: | ||
- 'Error return value of | ||
.((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). | ||
is not checked' | ||
- '(comment on exported (method|function|type|const)|should have( a | ||
package)? comment|comment should be of the form)' | ||
- 'func name will be used as test\.Test.* by other packages, and that | ||
stutters; consider calling this' | ||
- '(possible misuse of unsafe.Pointer|should have signature)' | ||
- 'SA4011' | ||
- 'G103: Use of unsafe calls should be audited' | ||
- 'G204: Subprocess launched with variable' | ||
- 'G104' | ||
- '(G301|G302|G307): Expect (directory permissions to be 0750|file | ||
permissions to be 0600) or less' | ||
- 'G304: Potential file inclusion via variable' | ||
- '(ST1000|ST1020|ST1021|ST1022)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Elvis339 these are the list of regex exclusions that were previously enforced by exclude-use-defaults
, but now have to be manually excluded.
To see the full list of default regex exclusions (including the revive
ones we're now enforcing), you can query via golangci-lint run --help
and scroll to the --exclude-use-default
flag
This PR applies the
exported
lint rule to thechain
package, which forces documentation on exported functions and interfaces.Long term, we will want to apply
exported
to all public facing packages, but this is a good starting point.