[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SgtCoDFish

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

The pull request process is described here

Hm, if we only run this in make and not in bazel, could this lead to either 1) bazel and make 'fighting' each other and 2) a change in what people need to run to do things?

It'd be great if goimports could be automatically called as part of {update,verify}-fmt.sh (and then work to remove that script depending on bazel?).

I'm worried that if we add things that only exist in Make, we end out in a 'split' state of the world until the migration from Bazel to Make is complete, where someone needs both build environments configured correctly (and drift between laptop go version and bazel go version could create issues). During k/k's migration, this didn't come up because they had always maintained both a Make-based and Bazel based build system, so our job is slightly harder than theirs in this instance 😬

Just my 2c 😄 (but a huge +1 to running goimports during presubmits!! 🎉)

During k/k's migration, this didn't come up because they had always maintained both a Make-based and Bazel based build system, so our job is slightly harder than theirs in this instance

That's useful history, thanks. 👍

Maintaining both make + bazel for cert-manager until the final switchover was actually my intention in any case, precisely to avoid the situation you describe with the two fighting.

I take the view that by only adding to make (and not going back and changing bazel) the worst that can happen is someone (justifiably) writes a PR locally using a bazel-based development flow in which all tests pass, and then is surprised to see goimports test failures after they raise the PR.

That's not ideal - ideally there's exactly one way to build code + run tests, which you're alluding to - but I think it's fine for the inbetween "dual-running" period before we do the final shift to using make entirely.

Certainly if the only difference until the switchover is that the make flow also runs goimports, that's totally acceptable to me because it's a small win and mostly we don't have issues with people not running goimports on their code before submitting it. Most of the work from now is going to be in replicating bazel stuff in make, not adding new stuff as in this PR.

(I think probably I should have an issue describing the dual running above, as a focal point for discussion, and I'll make that issue now)

I think this is probably good to go. If we really don't want to have goimports running without adding it into bazel, then we can not add make -f make/Makefile ci-presubmit to jetstack/testing. This PR is a no-op until we add it there.

/lgtm