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

node: Fix ineffectual assignment #14256

Merged
merged 1 commit into from
Dec 3, 2020
Merged

Conversation

brb
Copy link
Member

@brb brb commented Dec 3, 2020

/home/travis/gopath/src/github.com/cilium/cilium/pkg/datapath/linux/node_linux_test.go:1096:14: ineffectual assignment to err

Reported-by: André Martins andre@cilium.io
Fix: b78b3b7 ("node: Refcount neighbour entries")

/home/travis/gopath/src/github.com/cilium/cilium/pkg/datapath/linux/node_linux_test.go:1096:14:
ineffectual assignment to err

Reported-by: André Martins <andre@cilium.io>
Fix: b78b3b7 ("node: Refcount neighbour entries")
Signed-off-by: Martynas Pumputis <m@lambda.lt>
@brb brb added pending-review area/CI Continuous Integration testing issue or flake labels Dec 3, 2020
@brb brb requested a review from a team December 3, 2020 12:44
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Dec 3, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Dec 3, 2020
@brb brb requested a review from jrfastab December 3, 2020 12:44
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.1 Dec 3, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.6 Dec 3, 2020
@brb brb mentioned this pull request Dec 3, 2020
@aanm
Copy link
Member

aanm commented Dec 3, 2020

why os this only caught in 1.8 branch?

@aanm aanm added the release-note/misc This PR makes changes that have no direct user impact. label Dec 3, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 3, 2020
@sayboras
Copy link
Member

sayboras commented Dec 3, 2020

why os this only caught in 1.8 branch?

I think I can answer this question :). Basically, I removed make ineffassign before in favour of make lint with golangci-lint. ineffassign is configured in golangci-lint.yaml, but the build tags prevent it from scanning this file 😓

...
  build-tags:
    - !privileged_tests
...

Will update config file to prevent it in the furture.

@aanm aanm merged commit 354d42b into master Dec 3, 2020
@aanm aanm deleted the pr/brb/fix-ineffassign-arping branch December 3, 2020 14:10
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.8 in 1.8.6 Dec 3, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.8 in 1.8.6 Dec 3, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.1 Dec 4, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.1 Dec 4, 2020
@aanm aanm mentioned this pull request Dec 4, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.1 Dec 4, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.1 Dec 4, 2020
@aanm aanm mentioned this pull request Dec 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.8.6
Backport pending to v1.8
1.9.1
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

None yet

6 participants