✨ [golangci-lint] add local-prefixes to linters-settings of goimports

[tariq1890]

This PR adds a check to ensure that all first-party/local imports are moved to their own imports section

Sorting imports into groups helps with code readability. As per Go conventions, we maintain the following import groups:

• stdlib/built-in imports
• third-party imports
• local project imports

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: ac5e7c13cd537b1f5dc59a9863c390ca07773262

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: schrej, tariq1890
Once this PR has been reviewed and has the lgtm label, please assign alvaroaleman 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

[sbueringer]

Why?

[tariq1890]

I've updated the PR description

[sbueringer]

Can you please link the Go conventions you are referring to?

[tariq1890]

The Go style guide goes into this here. Plus, the goimports binary supports recognizing the local imports and enforces formatting accordingly. See here

[sbueringer]

The Go style guide goes into this here. Plus, the goimports binary supports recognizing the local imports and enforces formatting accordingly. See here

To quote the google style guide you are linking

Imports should be organized in two groups:

• Standard library packages
• Other (project and vendored) packages

It is acceptable to split the project packages into multiple groups if you want a separate group, as long as the groups have some meaning. Common reasons to do this:

• renamed imports
• packages imported for their side-effects

(So sounds like 3 groups would be "acceptable" for Google, but the "should" mentions 2 groups)

That the goimports binary supports to have local imports doesn't seem to indicate a strong perference towards having 3 groups.

[schrej]

Why do we have three groups on cluster-api?

[sbueringer]

Why do we have three groups on cluster-api?

Very good question. I think because at some point someone had an opinion about it (likely me 😂). But not necessarily because it's an established convention in Go.

[schrej]

So I took a look out of curiosity and you were the one that enabled gci, but didn't enable moving the local prefix into a separate group. That was done by stmcginnis, who also provided some reasoning for it:

Good practice is to have imports separated into three distinct groups:
standard library imports, third party package imports, and local
imports. The gci linter is used to enforce this convention, making it
easy to see what is local, what is standard with go, and what is being
pulled in externally.

The gci linter configuration has a "prefix" setting that is used to tell
it what to match as the starting string to identify local imports. This
was set to "false", effectively disabling the ability to differentiate
local imports from third party ones.

While it's not super important to have, I agree with it being easier to differentiate. Internally we're usually even splitting into four groups: std library, external, internal and module.

[sbueringer]

Thx for looking it up. I was mostly curious if there are actually Go conventions about this somewhere :)

I was preferring the split in 3 groups a while back, but over the years I realized that it makes absolutely 0 difference for me. I'm simply never manually reading over the imports in a way that the grouping would help.

That being said, just my personal opinion. No strong opinion either way

In general I have a preference for trying to not enforce personal preferences in Open Source projects, if it doesn't matter much either way.

@vincepri @alvaroaleman WDYT?

[alvaroaleman]

I don't really care much either way, I like to have stdlib in its own group but thats it

[tariq1890]

In general I have a preference for trying to not enforce personal preferences in Open Source projects, if it doesn't matter much either way.

I disagree with the characterisation that this is a "personal preference". This has been a common practice in Go for several years, it is exemplified by the fact goimports provides a -local flag and golangci-lint exposes a field for it in its config

[sbueringer]

Golangci-lint has a huge number of fields that allow all sorts of linter configurations. That doesn't make every one of them a best practice.

The fact that this is configurable and not the default in goimports makes it pretty clear to me that there is no clear consensus that this is the one standard for how to format imports.

I'm not saying that you are the only one who prefers this. I'm just saying there is no clear standard that everyone should or has to do it this way. Which makes it a personal preference for me

See also #3109 (comment)

Apparently nobody can point to a well known style guide that says folks should use 3 groups