Run revive linter in CI

[sspaink]

Run the revive linter for each pull request, but don't fail on it just output the results in a friendly format. Added a config toml with the rule "exported" removed that had 2K issues reported for things exported without a comment.

Once the errors are more cleaned up, this should be updated to warn/error if a pull request introduces issues. Related to the pr #8797.

[telegraf-tiger]

🤝 ✅ CLA has been signed. Thank you!

[zak-pawel]

LGTM!

[srebhan]

Not sure if this is the right place but I'll try anyway. ;-)
Working through #8797 I found the following things that we might want to have as a rule:

1. Do not use log in a plugin of type input, output, aggregator, processor (parser and serializer are special) but rather use telegraf.Logger.
2. Finding \"%s\" in fmt functions and use %q instead.
3. Patterns of the form

if condition {
    return true
}
return false

or the inverse can be simplified to return condition.

[zak-pawel]

Not sure if this is the right place but I'll try anyway. ;-)
Working through #8797 I found the following things that we might want to have as a rule:

1. Do not use log in a plugin of type input, output, aggregator, processor (parser and serializer are special) but rather use telegraf.Logger.
2. Finding \"%s\" in fmt functions and use %q instead.
3. Patterns of the form

if condition {
    return true
}
return false

or the inverse can be simplified to return condition.

@srebhan (and @ssoroka, @reimda): Unfortunately it seems that to add own rule, accepted PR to https://github.com/mgechev/revive or fork is needed: https://github.com/mgechev/revive#extensibility

[sspaink]

@zak-pawel that is an unfortunate limitation, maybe we should use golangci-lint for now which would let us create a Telegraf specific linter without having to push it upstream (more info). I can just update this PR to change from revive->golangci-lint.
We could potentially run both linters if we want or wait until golangci-lint supports running revive.

[p-zak]

@zak-pawel that is an unfortunate limitation, maybe we should use golangci-lint for now which would let us create a Telegraf specific linter without having to push it upstream (more info). I can just update this PR to change from revive->golangci-lint.
We could potentially run both linters if we want or wait until golangci-lint supports running revive.

I think that we can start with revive (as wew did :)) - because it is pretty good tool. We will try to fix as many problems as possible and after that (or in parallel) we can start to work with golangci-lint also.
In the meantime we need to think about custom rules.

[srebhan]

Perfect thanks!

[srebhan]

@sspaink I discussed with @p-zak and I think at least my requests can be covered. 1. can be checked by not allowing to include log in the plugins. 2. might be an upstream acceptable change anyway, same as 3. The probably also exist at least partially.

[p-zak]

@sspaink I discussed with @p-zak and I think at least my requests can be covered. 1. can be checked by not allowing to include log in the plugins. 2. might be an upstream acceptable change anyway, same as 3. The probably also exist at least partially.

@srebhan we can cover these in the future, they should not be part of this PR :)

[srebhan]

@p-zak I fully agree.

[zak-pawel]

@sspaink Don't we need to wait for next release (> v1.36.0) of golangci-lint to contain revive linter? Right now we have unknown linter: https://app.circleci.com/pipelines/github/influxdata/telegraf/3456/workflows/54ea1dc6-8eda-4b53-8dc5-d79ec57cba0b/jobs/78136

[zak-pawel]

@sspaink Don't we need to wait for next release (> v1.36.0) of golangci-lint to contain revive linter? Right now we have unknown linter: https://app.circleci.com/pipelines/github/influxdata/telegraf/3456/workflows/54ea1dc6-8eda-4b53-8dc5-d79ec57cba0b/jobs/78136

And v1.37.0 has been just released :)