chore: setup golangci-lint config file

[mmorel-35]

Description

This defines a golangci-lint config file

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mmorel-35
Once this PR has been reviewed and has the lgtm label, please assign ptabor for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ivanvc]

Thanks for the pull request, @mmorel-35. This is a great start, I suggest breaking this pull request into two (or at least two commits).

1. Add the .golangci.yaml file.
2. Enable the testifylint linter.

[ivanvc]

Do we want or need this file? We don't have it in any of our other repositories.

[mmorel-35]

Yes, you do because without the gofmt is fail on Windows platform.
I guess golangc-lint doesn't run on Windows with golangci-lint and gofmt/gofumpt enabled on the other repositories ?

[ivanvc]

Ah, this may be a fair point. I believe we don't have much support for Windows development.

I also know that @jmhbnz is not a fan of cluttering the top-level repositories. But this is a valid point. Do you think we should consider adding this file to other repositories? Or should we just add it to this repository, as it is the only one that runs workflows on Windows? Although technically, any new file created by a Windows contributor may have Windows line breaks.

[jmhbnz]

In our main repo we make it clear the only supported development environment for etcd is linux:

For both options, the only supported architecture is linux-amd64. Bug reports for other environments will generally be ignored. Supporting new environments requires the introduction of proper tests and maintainer support that is currently lacking in the etcd project.

https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#Set-up-development-environment

I defer to bbolt maintainers however on if that should apply for bbolt.

[ahrtr]

I have no strong opinion on this.

We don't officially support Windows (neither runtime env, nor development env), so I really don't want to spend time to handle anything related to Windows for now. So I prefer to letting contributors to configure their local git configuration.

We should be good as long as the linter (i.e gofmt) can identify such inconsistency (CRLF vs LF).

git config --global core.autocrlf <true|input>

[ahrtr]

We don't officially support Windows (neither runtime env, nor development env)

Sorry, it's true for etcd, but it might not be true for bbolt. bbolt isn't only used by etcd, it's also used in other use cases. But we can still expect contributors to configure their local git configuration as mentioned above.

We do support Windows.
https://github.com/etcd-io/bbolt/blob/main/bolt_windows.go

Then, the best option is not to run the golangci-lint in our Windows workflows.

Looks like I am missing something. Why the best option is not to run golangci-lint on Windows? Is it because it will definitely fail if gofmt is enabled? If Yes, WHY?

[ivanvc]

Looks like I am missing something. Why the best option is not to run golangci-lint on Windows? Is it because it will definitely fail if gofmt is enabled? If Yes, WHY?

We're already running the linter in the Linux workflows. The result shouldn't be platform-specific. So, if we run the workflow only on Linux, we wouldn't need to add the .gitattributes file.

[ivanvc]

Never mind. I thought that golangci-lint would run the linters over every go file in the project. After reading their documentation (and testing locally), I realized it only runs the linters for the host's platform (it doesn't even work with GOOS/GOARCH, ref: golangci/golangci-lint#5159).

So, following their documentation (you need to expand the "Multi OS example"), we need this .gitattributes file in the repository.

It made me realize that the only two architecture/OS files being linted are: bbolt_linux.go and bbolt_windows.go.

[ahrtr]

we need this .gitattributes file in the repository.

Adding .gitattributes works. But not adding the file will also work as long as we have linter to identify any such inconsistency (CRLF vs LF), please correct me if I am wrong. As mentioned above, we expect contributors to configure their local git config git config --global core.autocrlf <true|input>.

We don't want to add much much configuration files at the root directory, and add more maintain effort, although it's trivial for now.

[mmorel-35]

I believe that .gitattributes requires less effort than any other options that I know