Skip to content

Conversation

surajssd
Copy link
Contributor

The code that gets vendored by dep by default is huge. We can
leverage the functionality given by dep viz. prune. This will remove
all the non-go code and unused packages.

It makes an exception to the k8s.io/code-generator repository. Since
we use the scripts from this repository to generate code.

Ref: https://golang.github.io/dep/docs/Gopkg.toml.html#prune

The code that gets vendored by dep by default is huge. We can
leverage the functionality given by dep viz. prune. This will remove
all the non-go code and unused packages.

It makes an exception to the k8s.io/code-generator repository. Since
we use the scripts from this repository to generate code.

Ref: https://golang.github.io/dep/docs/Gopkg.toml.html#prune
@surajssd
Copy link
Contributor Author

cc: @hasbro17 @AlexNPavel

@hasbro17
Copy link
Contributor

@surajssd Thanks for PR.
There might be use cases where an operator relies on vendored non-go files like a script or something.
I don't think the SDK should enforce pruning those by default. Users can always prune those themselves if needed.
Pruning tests and unused pkgs by default seem okay.
WDYT?

@surajssd
Copy link
Contributor Author

@hasbro17 non-go also includes doc files like READMEs, etc.

IMHO we can always add an exception for the repos that need to have non-go stuff like in this PR:

[[prune.project]]
name = "k8s.io/code-generator"
non-go = false
unused-packages = false

@hasbro17
Copy link
Contributor

@surajssd I guess even without pruning non-go files, users already have to use the required tag to vendor non-go files from unused projects/pkgs.
So adding an exception for each such project along with that seems okay.

LGTM

@fanminshi
Copy link
Contributor

lgtm

@hasbro17 hasbro17 merged commit fedbab3 into operator-framework:master Aug 22, 2018
@surajssd surajssd deleted the remove-unnecessary-code branch August 23, 2018 06:06
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.

3 participants