Revive fixes - part 2 #8835

zak-pawel · 2021-02-08T19:36:43Z

Revive is fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint.

This is part2 of fixes for problems found for following set of rules:

[rule.if-return]
- Redundant if when returning an error. -> Checking if an error is nil to just after return the error or nil is redundant.
[rule.increment-decrement]
- Use i++ and i-- instead of i += 1 and i -= 1. -> By convention, for better readability, incrementing an integer variable by 1 is recommended to be done using the ++ operator. This rule spots expressions like i += 1 and i -= 1 and proposes to change them into i++ and i--.
[rule.var-declaration]
- Reduces redundancies around variable declaration. -> This rule proposes simplifications of variable declarations.
[rule.package-comments]
- Package commenting conventions. -> Packages should have comments. This rule warns on undocumented packages and when packages comments are detached to the package keyword. More information here
[rule.receiver-naming]
- Conventions around the naming of receivers. -> By convention, receiver names in a method should reflect their identity. For example, if the receiver is of type Parts, p is an adequate name for it. Contrary to other languages, it is not idiomatic to name receivers as this or self.
[rule.unexported-return]
- Warns when a public return is from unexported type. -> This rule warns when an exported function or method returns a value of an un-exported type.
- Few findings left untouched -> for some deeper review in the future.

For changed files, minor fixes were also made (got rid off "log" package where it was easy, CamelCase instead of snake_case, etc...)

[rule.if-return] [rule.increment-decrement] [rule.var-declaration] [rule.package-comments] [rule.receiver-naming] [rule.unexported-return]

telegraf-tiger

🤝 ✅ CLA has been signed. Thank you!

models/running_output.go

plugins/inputs/cpu/cpu.go

plugins/inputs/ipset/ipset.go

plugins/outputs/dynatrace/dynatrace.go

plugins/processors/topk/topk.go

srebhan

Looks good, found only very minor things... ;-)
Regarding the renaming of the "class" variable I'm not so sure about the exceptions, but if everybody can live with it...

srebhan

Looks good to me.

zak-pawel · 2021-02-11T17:47:29Z

For retry

zak-pawel · 2021-02-11T17:49:48Z

For retry of windows tests

agent/tick_test.go

models/running_output.go

plugins/common/shim/config.go

plugins/common/shim/config_test.go

plugins/inputs/passenger/passenger.go

plugins/inputs/webhooks/webhooks.go

ssoroka · 2021-02-16T16:05:46Z

I think there's still a lot of var struct/map/slice cases left over. I'm guessing we don't want to use rule.var-declaration.

zak-pawel · 2021-02-16T17:33:32Z

I think there's still a lot of var struct/map/slice cases left over. I'm guessing we don't want to use rule.var-declaration.

@ssoroka All var struct/map/slice cases reverted. rule.var-declaration is just fine, I've just changed too much initially.

* Revive fixes regarding following set of rules: [rule.if-return] [rule.increment-decrement] [rule.var-declaration] [rule.package-comments] [rule.receiver-naming] [rule.unexported-return]

* Revive fixes regarding following set of rules:

835cc1d

[rule.if-return] [rule.increment-decrement] [rule.var-declaration] [rule.package-comments] [rule.receiver-naming] [rule.unexported-return]

telegraf-tiger bot added the fix pr to fix corresponding bug label Feb 8, 2021

telegraf-tiger bot approved these changes Feb 8, 2021

View reviewed changes

uint64 correction

0896561