🌱 Update golint-ci, fix warnings

[benmoss]

What this PR does / why we need it:
Bumps golangci-lint. I noticed on #3107 that my editor was giving me some useful lint warnings that aren't being picked up by our CI, so I went to figure out why and discovered we're just a few versions behind.

I am ignoring these three linters since they produce new errors and I'm not sure if we want to fix/enforce them:

• godot: comments should end in a period
• goerr113: errors on dynamic error creation via errors.New and fmt.Errorf
• nestif: errors on deeply nested if statements

[detiber]

Should this return a 0 length FilterableMachineCollection here instead of nil? Not sure if there are any downstream effects of doing so.

[vincepri]

Was this reported by a linter?

[benmoss]

yeah there's a NPE immediately below this if you pass in a nil MachineList

[benmoss]

Maybe log.Fatal? I feel like guarding against nil pointers is almost the wrong thing to do, it seems like a programmer error if we're getting a nil as input here.

[vincepri]

If before we were throwing a panic, we can do the same here given that we don't return any errors. That said, if we were to remove the pointer, the function should still work like an empty list?

[detiber]

It might be better to add a variable for the initialization size for the make call below, or to initialize it as 0 length. I believe ass this stands today if one were to pass a nil pointer and they tried to use the FilterableMachineCollection returned by this function and later tried to use the Insert method, they would get a panic on Insert.

This should accomplish what looks like the original intent here:

	var initLength int
	if machineList != nil {
		initLength = len(machineList.Items)
	}
	ss := make(FilterableMachineCollection, initLength)

[benmoss]

Turns out if we just remove the nil check that used to happen here the linter doesn't care anymore. This will result in a panic which I still think is the right behavior. I think the intention of the linter is just to show you that if you want to guard against nils, this code was failing to properly do so. If we want to just let it fail (since it's not an actual feature of the code that we want to support passing around nil MachineLists) then we do nothing.

[vincepri]

+1 to updating to the latest version, although I'm not in favor of getting so many changes in while we're in v0.3.x — some of which seem to have side effects or change logic.

Can we scope down the changes to the minimum necessary? Let's add the ignored patterns to the excluded list and open issues to deal with it later.

[fabriziopandini]

do you have any context on these file permissions ones?

TBH no. Most of them are in test, we can start to get them fixed

[benmoss]

TBH no. Most of them are in test, we can start to get them fixed

Yeah I realized I think we can actually update all of these without any impact on the code, they are just writing files for the tests to consume but I don't the perms need to be as wide as they are.

+1 to updating to the latest version, although I'm not in favor of getting so many changes in while we're in v0.3.x — some of which seem to have side effects or change logic.

Can we scope down the changes to the minimum necessary? Let's add the ignored patterns to the excluded list and open issues to deal with it later.

Which are the minimum necessary? None of these seem like dangerous changes to me. We can also just hold off until after 0.3?

[wfernandes]

@benmoss I do not have much context on the file permissions but I don't see the harm in changing them and making sure they run fine on CI. I'm sure it just started somewhere and then copy-pasta elsewhere.

[vincepri]

/approve
/milestone v0.3.7

@benmoss Good work here, last changes look good to me. Can we re-categorize as running given that this isn't a user-facing feature or major change?

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benmoss, vincepri

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 [vincepri]

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

[wfernandes]

/lgtm