8000 hubble, policy: shuffle types to reduce imports by squeed · Pull Request #32378 · cilium/cilium · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

hubble, policy: shuffle types to reduce imports #32378

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

Merged
merged 1 commit into from
May 14, 2024

Conversation

squeed
Copy link
Contributor
@squeed squeed commented May 6, 2024

The "leaf" type pkg/hubble/api/v1 inadvertently imported pkg/policy, which has a large set of imports. This causes a few problems, most notably the hubble CLI binary to have a huge number of unnecessary imports. This also means a large set of the codebase must build on all architectures, unnecessarily to boot.

So, this commit has two changes:

  1. Lift the policy Key type in to its own package. Since this is referenced everywhere, use a type alias to keep our sanity.

  2. Move the hubble EndpointInfo interface out of the API type, since it's not really relevant to the API package.

This reduces the number of local imports in the hubble binary from 196 to 57. A nice cleanup.

@squeed squeed added the release-note/misc This PR makes changes that have no direct user impact. label May 6, 2024
@squeed squeed requested review from a team as code owners May 6, 2024 11:36
@squeed squeed requested review from christarazi and lambdanis May 6, 2024 11:36
Copy link
Member
@gandro gandro left a comment

Choose a reason for hiding this comment

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

Nice cleanup! Note that due to the package move, there are some lint failures now:
image

Otherwise, this looks good to me!

@squeed squeed force-pushed the hubble-trim-types branch from ccecfce to df41b90 Compare May 6, 2024 12:21
@squeed squeed requested a review from a team as a code owner May 6, 2024 12:21
@squeed
Copy link
8000
Contributor Author
squeed commented May 6, 2024

Note that due to the package move, there are some lint failures now:

Hah. I had to actually force-disable that check in .golangci.yaml, which is a bummer but better than making hundreds of invocations harder to read.

@squeed
Copy link
Contributor Author
squeed commented May 6, 2024

/test

@squeed squeed force-pushed the hubble-trim-types branch from df41b90 to 29e4e44 Compare May 7, 2024 12:40
@squeed
Copy link
Contributor Author
squeed commented May 7, 2024

I gave up and just fixed the struct literals; we use bot govet linter as well as go vet, so configuring them is not possible :-/

@squeed
Copy link
Contributor Author
squeed commented May 7, 2024

/test

1 similar comment
@squeed
Copy link
Contributor Author
squeed commented May 8, 2024

/test

The "leaf" type pkg/hubble/api/v1 inadvertently imported pkg/policy,
which has a large set of imports. This causes a few problems, most
notably the hubble CLI binary to have a huge number of unnecessary
imports. This also means a large set of the codebase must build on all
architectures, unnecessarily to boot.

So, this commit has two changes:

1. Lift the policy Key type in to its own package. Since this is
   referenced **everywhere**, use a type alias to keep our sanity.

2. Move the hubble EndpointInfo interface out of the API type, since
   it's not really relevant to the API package.

This reduces the number of local imports from 196 to 57. A nice cleanup.

Signed-off-by: Casey Callendrello <cdc@isovalent.com>
@squeed squeed force-pushed the hubble-trim-types branch from 29e4e44 to 0c1823d Compare May 8, 2024 15:27
@squeed
Copy link
Contributor Author
squeed commented May 8, 2024

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 11, 2024
@squeed squeed added this pull request to the merge queue May 14, 2024
Merged via the queue into cilium:main with commit 93f8bae May 14, 2024
63 of 64 checks passed
@squeed squeed deleted the hubble-trim-types branch May 14, 2024 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. 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.

5 participants
0