[v5.2] Backport a number of lint fixes #23704
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes new spotted issues around printf() formats and using os.Setenv() in tests. Signed-off-by: Paul Holzinger <[email protected]>
The new golangci-lint version 1.60.1 has problems with typecheck when linting remote files. We have certain pakcages that should never be inlcuded in remote but the typecheck tries to compile all of them but this never works and it seems to ignore the exclude files we gave it. To fix this the proper way is to mark all packages we only use locally with !remote tags. This is a bit ugly but more correct. I also moved the DecodeChanges() code around as it is called from the client so the handles package which should only be remote doesn't really fit anyway. Signed-off-by: Paul Holzinger <[email protected]>
Now that we have propert !remote tags set everywhere we can just rely on that and do not need to skip any dirs. Also on linux do not lint three times, one remote run is enough. We still have to skip the test dir for windows/macos though or we need to add linux build tags there everywhere as well. This seems simpler. Signed-off-by: Paul Holzinger <[email protected]>
We need something newer than 4.14 anyway now for most Podman functions. This is breaking liniting on windows as the function doesn't work there. Signed-off-by: Paul Holzinger <[email protected]>
It qemu cannot be compiled anyway so make sure we do not try to compile parts where the typechecker complains about on windows. Also all the e2e test files are only used on linux as well. pkg/machine/wsl also reports some error but to many for me to fix them now. One minor problem was fixed in pkg/machine/machine_windows.go. Signed-off-by: Paul Holzinger <[email protected]>
Fix one minor issue with vfkit error handling. First checking if err != nil OR errors.Is() is pointless as the err != is already true. Second nilerr complains because we return nil when we hit an error branch, in this case this is correct because an error means VM is stopped. Signed-off-by: Paul Holzinger <[email protected]>
TomSweeneyRedHat
added
the
No New Tests
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: TomSweeneyRedHat 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:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
github-actions
bot
added
machine
kind/api-change
|
"Build for MAcOS" has completed successfully |
|
/lgtm |
|
Closing, will cherry-pick to #23699 |
stale-locking-app
bot
added
the
locked - please file new issue/PR
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry pick a number of lint issue from #23614, hopefully the mac one of most import.
Does this PR introduce a user-facing change?