search

kubernetes-sigs / kueue

Kubernetes-native Job Queueing
https://kueue.sigs.k8s.io
Apache License 2.0
1.24k stars 222 forks source link

Reformat all imports using gci linter #2198

Open alexandear opened 1 month ago

alexandear commented 1 month ago

What would you like to be cleaned:

Reformat all imports using gci linter once the issue daixiang0/gci#135 is resolved.

Disable goimports because gci does the same work.

Why is this needed:

gci ensures consistency of the import order.

See discussions in #2069 and #2184.

trasc commented 1 month ago

Maybe I'm missing something, why do we need this gci ?

alexandear commented 1 month ago

We need gci because it was introduced by PR #2069.

alculquicondor commented 5 days ago

If the PR merged, doesn't it mean that we have no violations?

alexandear commented 4 days ago

If the PR merged, doesn't it mean that we have no violations?

The PR #2069 didn't enable gci due to incorrect configuration, but the author and reviewers thought it was enabled. See the description in #2184 for a detailed explanation.

alculquicondor commented 4 days ago

gotcha, so we can't re-enable gci just yet.

OTOH, we don't really use kube-builder scaffolding.

How reliable is this gci linter? The fact that it belongs to a person's github instead of an organization seems a bit worrisome.

alexandear commented 4 days ago

gci has been included in golangci-lint since v1.30.0, so it's quite stable.

But let's ask the author of the original PR who wants to enable gci. @vladikkuzn, is it worth replacing goimports with gci?

vladikkuzn commented 20 hours ago

My opinion is that if it's stable with comments (which is the issue I didn't caught unfortunately) and it won't cause any further problems, then yes. It would make imports ordered the way everybody orders them, but automatically