-
Notifications
You must be signed in to change notification settings - Fork 43
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
QA: update code for golangci-lint and run go fmt
.
#87
Conversation
plugins/nat/nat_windows.go
Outdated
nm network.Manager | ||
} | ||
|
||
type netPlugin struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is shifted right which is not correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's actually go fmt
identifying code snippets within the comment and attempting to indent it so it can get formatted as code by godoc
.
Either way, I've noticed that these comments, initially added in this PR, are most likely redundant since the netPlugin
struct has been moved into its own package, so I'll be removing them completely.
5877c01
to
526ea4a
Compare
Signed-off-by: Nashwan Azhari <[email protected]>
Signed-off-by: Nashwan Azhari <[email protected]>
526ea4a
to
9e3c86b
Compare
While the vast majority of the changes are trivial, I'd like to draw the reviewers' attention to the following places where semantics may have slightly changed and I request you take a closer look at:
strings.EqualFold()
which has the potential to do something weird on non EN-US systems, but considering it's only used during testing and only seems to be called during tests so it should be a real-world issue.plugin_testing.RunAll()
since both the sub-calls toplugin_testing.{RunUnitTest,RunBasicConnectivityTest}
in turn already callSetup()/Teardown()
.