Elastic linter

[ewgRa]

PR for elastic coding style checks (#6273), now support check of - "no new line before error check" as only one well known and described.

Some demo of results, how it works with comments, that was created by CI:
https://github.com/ewgRa/beats/pull/3
https://github.com/ewgRa/beats/pull/2

Some readme (not so friendly now, but more or less give explanations): https://github.com/ewgRa/gocsfixer/blob/master/README.md

For usage, there GOCSFIXER_GITHUB_TOKEN env must be set in travis repository settings, it is "Access Key" with public_repo rights.

[elasticmachine]

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

[ruflin]

Thanks for putting all the efforts into this. Two things I really like that this is a generic project which can be applied to any other project and that it can be run locally or on CI in the same way as far as I understand.

Do you already use the project in one of your go projects?

[ewgRa]

@ruflin

that it can be run locally or on CI in the same way

I think you know PHP-CS-Fixer project, gocsfixer - kind of the same, but for Go. This is main idea. Yes, it can be run on CI and locally for checking and fixing. I try to make it flexible as much as possible, to fit future requirements from other projects and companies, that can be interested in this tool.

Do you already use the project in one of your go projects?

No. I have super small go projects (mostly pet projects) and there is no coding style conventions.

I think adopt it in beats first, have feedback, make changes if needed. After that introduce it to local go community as next step. At this step maybe introduce good commenting process for specific PR line, like "hound" doing it.

Why are you asking for? Chicken-egg problem? :)

[ewgRa]

Inline comments from bot added, example: https://github.com/ewgRa/beats/pull/5#pullrequestreview-126981133

[ruflin]

@ewgRa Thanks for keep pushing this. It's still on my list to review and think about but there were just too many more urgent priorities recently.

[ruflin]

Recently @urso started to work on https://github.com/urso/beatsfmt because of our new lincese headers. I could see this projects overlapping.

[ewgRa]

@ruflin kind of, maybe how @urso implement it - specific for Beats project and differ from what I propose here. With some effort I think it can be done in one way, especially if you will provide task definition, what you want to achieve.

I implement new generic "File header" linter, that can cover similar requirement for license header.
Here is example of work https://github.com/ewgRa/beats/pull/6#pullrequestreview-134185213 and implementation ewgRa/gocsfixer@e87242c#diff-862ea411731cab9484a81b538b5f5c62

[ewgRa]

Looks like there is will be another linter, "cli" one. That for lint or fix will perform some cli commands. For example @urso case can be done by several unix commands. Also as "File header" linter can be achived with combination of "cat", "diff", "head" etc.

[ruflin]

I think @urso also has in mind to make it more generic why I think long term the projects could be getting closer to each other.

One thing that I think is important for any "fixer" is that it's cross platform so it also work on Windows systems.

[ewgRa]

Ok, I'm finished fixers, that I face during contribution to Beats. I think it is good milestone for next step.

I will polish a little bit documentation and within one-two month will prepare presentation for local golang meetup.

I would be happy if close to this time gocsfixer will be adopted in Beats, but I don't see what else I can do from my side. I'm ready to discuss it, answer on questions, help where I can.
If it will be adopted - it will be good use case for presentation. If not - nothing critical,
I think if gocsfixer can bring profit to developers - they will be interested anyway. If not, then it will be just experience for me :)

For now on gocsfixer support five fixers, mostly oriented on Beats cases:

1. Alternative call: can cover case "Instead of logp.Warn use logp.Err"

2. File header

3. Group standard imports, like:

import (
    "os"
    "github.com/foo/bar"
    "testing"
)

will be

import (
    "os"
    "testing"

    "github.com/foo/bar"
)

4. No new line before error check:

err = test()

if err == nil {
}

will be

err = test()
if err == nil {
}

5. Use path.Join: os.Readlink("foo/bar") will be os.Readlink(path.Join("foo", "bar"))

[ruflin]

Thanks for keeping pushing the project forward. The main issue on our end is time and priority. So far this didn't make it to the top of our priority list. We will get back to you when we find the time for it.

[bot-ci]

Thanks for PR. Please check results of automatic CI checks

[bot-ci]

[recommendation] No new line before error check

[bot-ci]

[recommendation] No new line before error check

[bot-ci]

[recommendation] No new line before error check

[bot-ci]

[recommendation] Group stdLib imports

[bot-ci]

[recommendation] Group stdLib imports

[bot-ci]

[recommendation] Group stdLib imports

[bot-ci]

[recommendation] Group stdLib imports

[bot-ci]

[recommendation] Group stdLib imports

[ewgRa]

Also there is two interesting projects, that also related with this topic.

One well known https://github.com/alecthomas/gometalinter , that would be interesting to think about integrate with gocsfixer.

And another one https://github.com/go-critic/go-critic , that potentially can replace gocsfixer. I start discussion go-critic/go-critic#554 to understand is it possible, or at least can gocsfixer reuse something from go-critic, or put there some linters.

[leehinman]

@ewgRa Thank you for all the hard work on the project but getting a linter integrated hasn't been a top priority. If we decide to add a linter in the future we will definitely look at gocsfixer.