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

Remove unused funcs, types and global vars #10085

Merged
merged 18 commits into from
Feb 20, 2020

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Feb 7, 2020

Remove some unused funcs, types and global vars all over the place, found by running staticcheck -check U1000.

before:

   text    data     bss     dec     hex filename
55876285         893753 7752192 64522230        3d887f6 daemon/cilium-agent
57015715         610160  211608 57837483        37287ab operator/cilium-operator
35449857         486256  192536 36128649        2274789 plugins/cilium-cni/cilium-cni

after:

   text    data     bss     dec     hex filename
55874194         893689 7752128 64520011        3d87f4b daemon/cilium-agent
57014951         610160  211576 57836687        372848f operator/cilium-operator
35449696         486256  192536 36128488        22746e8 plugins/cilium-cni/cilium-cni

No big wins here for #10056 yet.


This change is Reviewable

@tklauser tklauser requested review from a team February 7, 2020 10:09
@tklauser tklauser requested review from a team as code owners February 7, 2020 10:09
@tklauser tklauser requested review from a team February 7, 2020 10:09
@tklauser tklauser force-pushed the pr/tklauser/binsize-reduce-unused branch from 228aa37 to cc3fd11 Compare February 7, 2020 10:20
@maintainer-s-little-helper
Copy link

Release note label not set, please set the appropriate release note.

@tklauser tklauser added release-note/misc This PR makes changes that have no direct user impact. and removed dont-merge/needs-release-note labels Feb 7, 2020
@tklauser
Copy link
Member Author

tklauser commented Feb 7, 2020

test-me-please

@coveralls
Copy link

coveralls commented Feb 7, 2020

Coverage Status

Coverage increased (+0.03%) to 44.613% when pulling da72060 on pr/tklauser/binsize-reduce-unused into f912be7 on master.

@tklauser tklauser force-pushed the pr/tklauser/binsize-reduce-unused branch from cc3fd11 to db75818 Compare February 7, 2020 14:33
@tklauser tklauser requested review from a team as code owners February 7, 2020 14:33
@tklauser
Copy link
Member Author

tklauser commented Feb 7, 2020

test-me-please

@aanm aanm added the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Feb 7, 2020
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Bits from my codeowners LGTM.

clientPkg "github.com/cilium/cilium/pkg/client"
healthClientPkg "github.com/cilium/cilium/pkg/health/client"

"github.com/prometheus/client_golang/prometheus"
log "github.com/sirupsen/logrus"
)

const (
updateLatencyMetricsInterval = 30 * time.Second
Copy link
Member

Choose a reason for hiding this comment

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

🤔

Copy link
Member

Choose a reason for hiding this comment

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

Seems that the const has not been used since the beginning: #4437 (also git log -S updateLatencyMetricsInterval).

@aanm aanm added this to the 1.8 milestone Feb 18, 2020
@aanm aanm removed the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Feb 18, 2020
@aanm
Copy link
Member

aanm commented Feb 18, 2020

@tklauser needs rebase

@aanm aanm added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Feb 19, 2020
@tklauser tklauser force-pushed the pr/tklauser/binsize-reduce-unused branch from db75818 to 6173a27 Compare February 19, 2020 09:19
@tklauser tklauser removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Feb 19, 2020
@tklauser
Copy link
Member Author

test-me-please

1 similar comment
@tklauser
Copy link
Member Author

test-me-please

The `log` var is unused in some packages, so remove it where this is the
case.

Signed-off-by: Tobias Klauser <[email protected]>
This is unused since commit 1121202 ("fqdn: L3-aware L7 DNS policy enforcement")

Signed-off-by: Tobias Klauser <[email protected]>
Also reorder initialization in NewDatapath so that embedded types are
initialized first.

Signed-off-by: Tobias Klauser <[email protected]>
(*linuxNodeHandler).lookupDirectRoute is only used in test code and does
not use any members of linuxNodeHandler, so make it a regular function
and move it to node_linux_test.go

Signed-off-by: Tobias Klauser <[email protected]>
type EndpointPolicyVisibilityEventResult, (*Endpoint).getIDandLabels and
(*Endpoint).removeProxyRedirect are unused, remove them.

Signed-off-by: Tobias Klauser <[email protected]>
Remove unused type rulesManager and func checkLocks.

Signed-off-by: Tobias Klauser <[email protected]>
Signed-off-by: Tobias Klauser <[email protected]>
@tklauser tklauser force-pushed the pr/tklauser/binsize-reduce-unused branch from 6173a27 to da72060 Compare February 19, 2020 15:08
@tklauser
Copy link
Member Author

test-me-please

1 similar comment
@tklauser
Copy link
Member Author

test-me-please

@borkmann borkmann merged commit ececa5e into master Feb 20, 2020
@borkmann borkmann deleted the pr/tklauser/binsize-reduce-unused branch February 20, 2020 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants