Update gocritic enabled/disabled checks to fix linter

[peterjan]

This PR adjusts our linter so it passes again after upgrading golangci to v1.57.0.

[lukechampine]

sorry to be a curmudgeon but I disagree with all three of these style rules lmao 😅

[peterjan]

curmudgeon

Haha, I don't mind at all Luke. I only wanted to apply the same linters we have in renterd because Chris told me we'll move towards centralising these configs soon. Since the changes were minimal I figured to just open the PR. For what it's worth though, I kind of like initClause. The other two I don't really care about, especially nestingReduce.

Edit: I wanted to add that even though I agree these rules are often silly, I am very much in favour of adding them because they unify our codebase. For me, there's nothing worse than reading through a codebase and seeing who wrote it. It's just annoying for the reader. I think Chris and I have evolved to develop a very similar style but we can not agree on capitalising comments it seems. I write // add 3 test hosts and Chris will write // Add 3 test hosts. (can you believe that). It sounds silly but if I touch a test, I have to go in and rewrite all of those because I can't handle two styles of comments in a single test/method/function leading to merge conflicts etc. So I don't really care all that much which ones we enable/disable, but I do think it would be great to have the same set of them applied across all of our repos.

[lukechampine]

agreed, consistency is very important. If I didn't believe that, I would have given up on Go a long time ago. :P

gofmt is good because it makes code easier to scan. You don't need to spend mental energy adjusting to someone else's formatting style.

govet is good because it only flags code that is highly likely to have correctness issues.

gocritic and other linters occupy a weird in-between space. Like gofmt , they aren't trying to find bugs, just style violations. But unlike gofmt, they (typically) can't fix those problems automatically. It's annoying when CI flags your code for a linter violation, so the programmer has to develop a mental model of all the rules.

If the linter could auto-fix everything, I probably wouldn't care about it much. Sometimes gofmt formats things in a way I don't like -- but it runs on every save! So if I get cute with my formatting, and gofmt clobbers it, I just sigh and move on; it's not worth the trouble. But linters make you do the refactoring.

Perhaps a better approach to linting would be to run the linter just once, every few months, and manually look through the flagged violations and decide if they're worth fixing. That way, we can get most of the benefits without being henpecked on every CI push :P

[ChrisSchinnerl]

To be fair the fact that it wasn't clear whether memory should be reused or not from a quick glance is probably enough reason to keep that linter rule and use the more traditional pattern to make that clear.

I could probably get behind the other patterns even though deleting in an if seems odd at first but at least it's super obvious and might not hide unexpected behaviour^^

[peterjan]

I don't want to make this PR the PR where we discuss what checks we should enable/disable. Can we do that when we centralise the configs? I reverted all changes only to make this PR all about getting master to pass CI.

[ChrisSchinnerl]

I think we should still keep the append linter but I don't mind dropping the other ones.

Definitely not in favour of running linters only once a month cause

1. nobody ever checks the result of scheduled actions
2. linting once a month is like saying you'll clean your apartment once a month instead of putting stuff away right away. You'll just build up a mess that is too annoying to fix.