Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix golangci-lint issues #1480

Merged
merged 5 commits into from
Aug 16, 2022
Merged

Fix golangci-lint issues #1480

merged 5 commits into from
Aug 16, 2022

Conversation

dcantah
Copy link
Contributor

@dcantah dcantah commented Aug 12, 2022

With an upgrade to our linter comes a lot of new yelling at us. This does quite a bit:

  • Removes all usage of io/ioutil as it's been deprecated since 1.16. This additionally moves some tests to use t.TempDir() which was a nice side effect.
  • Move CopyFileW usage in /internal/copyfile to winapi so it can be generated
    and not call syscall.Syscall directly.
  • gofmt -s everything our linter is not happy about.
  • Skip linting net.Error.Temporary usage for now. net.Error's temporary method is deprecated since 1.18 but we make sure HcsError, ProcessError and SystemError all adhere to net.Error's interface which contains the Temporary method. We're reworking the hcs errors package shortly and I'm not sure of the history or significance behind that choice. This seems sane for now.
  • Don't whine about gofmt issues for the generated schema files
  • Remove the skip-go-installation flag from our golanci-lint action as it no longer exists.
  • Swap to setup-go v3 action

@dcantah dcantah requested a review from a team as a code owner August 12, 2022 12:25
@helsaawy helsaawy self-assigned this Aug 12, 2022
Copy link
Contributor

@helsaawy helsaawy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor doc comment formatting issues

cmd/containerd-shim-runhcs-v1/task_hcs.go Outdated Show resolved Hide resolved
internal/guest/prot/protocol.go Show resolved Hide resolved
internal/guest/storage/devicemapper/targets.go Outdated Show resolved Hide resolved
internal/guest/transport/transport.go Outdated Show resolved Hide resolved
internal/layers/layers.go Show resolved Hide resolved
internal/uvm/network.go Outdated Show resolved Hide resolved
pkg/securitypolicy/securitypolicy.go Outdated Show resolved Hide resolved
ext4/tar2ext4/tar2ext4.go Outdated Show resolved Hide resolved
Copy link
Contributor

@anmaxvl anmaxvl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. did gofmt mangle the comments in a few places?

internal/guest/prot/protocol.go Outdated Show resolved Hide resolved
internal/guest/storage/devicemapper/devicemapper.go Outdated Show resolved Hide resolved
internal/tools/networkagent/main.go Show resolved Hide resolved
@kevpar
Copy link
Member

kevpar commented Aug 12, 2022

So obviously there's a ton of messed up comment formatting. Are you still investigating this?

@dcantah
Copy link
Contributor Author

dcantah commented Aug 12, 2022

Yea.. I thought this was on draft if I'm being honest 😿Hamza's comments let me know this was not the case haha. I haven't looked at this since this morning, but I'm looking now. Need to find what gofmt doesn't like anymore

@dcantah dcantah force-pushed the fix-lint-issues branch 3 times, most recently from 2c634c1 to c09675e Compare August 14, 2022 01:51
@dcantah
Copy link
Contributor Author

dcantah commented Aug 15, 2022

@kevpar @anmaxvl Fixed the weird formatting for comments up to match the new 1.19 gofmt behavior for doc comments. I'll squash the extra pr feedback comments into their rightful commit at the end

Gets rid of io/ioutil usage in favor of the os and io replacements.
ioutil has been deprecated since 1.16.

This additionally starts to use t.TestDir() in some tests instead which
is a nice side effect.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
We had a stray use of syscall.Syscall that should be in winapi with the
rest of our defs.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
We're reworking the hcs/errors package soon, but in the meantime
don't lint the use of the deprecated err.Temporary.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
gofmt -s on go 1.19

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
v3 golangci-lint action removed the skip-go-installation flag
and now explicitly requires using the setup-go action to function.

- Get rid of skip-go-installation
- Swap to v3 of setup-go
- Remove only-new-issues usage

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@dcantah
Copy link
Contributor Author

dcantah commented Aug 16, 2022

Squashed, will wait for CI to finish again

@dcantah dcantah merged commit 298b31d into microsoft:main Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants