Run gofmt via golangci-lint

[ferhatelmas]

Drop custom script since golangci-lint can already do it.

[codecov-io]

Codecov Report

Merging #425 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #425   +/-   ##
=======================================
  Coverage   56.41%   56.41%           
=======================================
  Files          19       19           
  Lines         927      927           
=======================================
  Hits          523      523           
  Misses        349      349           
  Partials       55       55

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab1c7f7...6d99b6e. Read the comment docs.

[corneliusweig]

I think we can just drop the extra script. There is no need to enable gofmt, because we already do goimports.

A notable difference between the original setup and the suggested one is that the script gives a little more context. For example:

• goimports via golangci-lint:

cmd/krew/cmd/search.go:92: File is not `goimports`-ed with -local sigs.k8s.io/krew (goimports)
                  rows = sortByFirstColumn(rows)

• gofmt via script

Gofmt errors in files:
./cmd/krew/cmd/search.go
diff -u ./cmd/krew/cmd/search.go.orig ./cmd/krew/cmd/search.go
--- ./cmd/krew/cmd/search.go.orig       2019-12-04 20:42:47.010461167 +0100
+++ ./cmd/krew/cmd/search.go    2019-12-04 20:42:47.010461167 +0100
@@ -89,7 +89,7 @@
                        }
                        rows = append(rows, []string{name, limitString(plugin.Spec.ShortDescription, 50), status})
                }
-                 rows = sortByFirstColumn(rows)
+               rows = sortByFirstColumn(rows)
                return printTable(os.Stdout, cols, rows)
        },
        PreRunE: checkIndex,

I find the latter a little easier to understand, but it's not a big deal.

[corneliusweig]

I think we don't even need this. Looking at golangci-lint linters | grep goimports:

goimports: Goimports does everything that gofmt does. Additionally it checks unused imports [fast: true, auto-fix: true]

[ferhatelmas]

not exactly correct.

goimports version of golangci doesn't do simplification.

[ferhatelmas]

here is the configuration: https://github.com/golangci/golangci-lint/blob/master/pkg/config/config.go#L139

maybe we should suggest adding it then goimports can be used solely.

[corneliusweig]

Mm.. looks like goimports doesn't do simplifications: golang/go#21476

[ferhatelmas]

There was a long standing demand, I thought it was already added but it seems it's still missing.

[ahmetb]

I find the latter a little easier to understand, but it's not a big deal.

Agreed.

[corneliusweig]

So I guess this is indeed the best simplification for gofmt. From my side
/lgtm

[ferhatelmas]

@ahmetb I believe this is waiting for your review/approval.

[ahmetb]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahmetb, ferhatelmas

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [ahmetb]

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