chore: Bump golangci-lint and fix linter issues

[lindnerby]

Description

Changes proposed in this pull request:

• bump version to 2.3.1
• new linter noinlinerr produces quite a bit of a changelog, all occurences are fixed
  Related issue(s)

[ruanxin]

Why does this inlined error handling become not recommended?

[lindnerby]

There are two main reasons:

• First, readability: Many parts where this construct is used with longer functions and multiple return values make the code barely readable. Draws the attention back to the function and the error handling becomes more explicit and readable.
• Second: field shadowing. Due to this construct there are often a re-declarations of the err variable, which is unnecessary.
  Also the usage is very inconsistent, sometimes it's used, sometimes not.
  I really like this additional linter, improves the column width so especially for the habit of multiple split editor window arrangements, code fits nicely without needing to scroll to the right all the time 😄

[ruanxin]

To be honest, I hold a different opinion. The noinlinerr rule completely overlooks one important advantage of inline error handling: it limits the scope of the error variable to exactly where it’s relevant.

When an error variable is only used for a single operation, there’s no reason to keep it alive in a wider scope. Inline error handling ensures that the variable is tied only to the operation that produced it, reducing the chance of accidentally reusing it or mixing it up with errors from unrelated logic.

All the code refacted in this PR immeately lost such benefit, for example:

lifecycle-manager/internal/controller/kyma/controller.go

Line 285 in 6b11294

if err := r.syncStatusToRemote(ctx, kyma); err != nil {

, the err is scoped only to syncStatusToRemote, avoiding any confusion with errors elsewhere in the function.

However, in this PR:
https://github.com/kyma-project/lifecycle-manager/pull/2644/files#diff-4d414901850a616d4e06a120615896904639aef683f0a4d8860abf65c76179d4R299

the change implicitly overwrites the error from err = r.replaceSpecFromRemote(ctx, kyma). This kind of overwrite significantly reduces readability I would say, because now you must track a single err variable declared at the top level across multiple unrelated operations.

Example of the problem:

err := doSomething()
if err != nil {
    return err
}

err = doAnotherThing()
if err != nil {
    return err
}

err = doLastThing()
if err != nil {
    return err
}

Here, the single err is reused across unrelated operations, which makes it harder to scan visually and reason about which line actually caused the failure.

Also, big company like Google, Uber in their coding styles explictely mentioned this inline error is good and recommend to follow:

Google code style - Concision
Uber - Reduce Scope of Variables

Given this, I don’t think we should introduce noinlinerr without any debate. I’ll also be leaving my argument in the upstream PR: golangci/golangci-lint#5826

[lindnerby]

Valid point. Then let's open this up for debate in next Arch Round 👍

[lindnerby]

As discussed in Architecture Round, we don't want the noinlineerr enable and consider it bad style to enforce not using it.
Closing this and opening another for the remaining linter introduction.