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 vendor #422

Merged
merged 2 commits into from
May 15, 2019
Merged

Conversation

DirectXMan12
Copy link
Contributor

Modules don't need vendor any more, so it doesn't serve any purpose.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 13, 2019
@k8s-ci-robot k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/auth Categorizes an issue or PR as relevant to SIG Auth. labels May 13, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DirectXMan12

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 13, 2019
@tamalsaha
Copy link
Contributor

I wonder if the vendor folder should be preserved until GOPROXY becomes common/stable and Go 1.13 is released (when go mod becomes default on). I saw that prometheus-operator project uses go build -mod=vendor to keep using the vendor folder.

@DirectXMan12
Copy link
Contributor Author

yeah, that's what we had in controller-tools before, but I'm not convinced of how useful it is. What's the usecase?

@tamalsaha
Copy link
Contributor

Vendoring is useful for folks who are behind slow internet connection. This feels a bit rushed. But I have no objection.

@tamalsaha
Copy link
Contributor

Another usefulness of vendor folder is to see what has changed when go.mod file is updated. The version syntax of go.mod is unreadable for k8s.io repos. So that might be useful at times. But that can be done locally, I guess.

@DirectXMan12
Copy link
Contributor Author

Vendoring is useful for folks who are behind slow internet connection

I am sympathetic to that, but ideally the local shared module cache should help with that (and, not checking in vendor helps keep the overall repo size down). Additionally, the vendor directory won't actually help if you're consuming controller-runtime, just if you're hacking on CR itself, so for users with slow internet, it doesn't matter either way.

The version syntax of go.mod is unreadable for k8s.io repos

Yeah, that part's fairly unfortunate, but there's not really much we can do until k/k gets its act together.

Modules don't need vendor any more, so it doesn't serve any purpose.
This caches the go mod and go build cache directories, which should
speed up test runs.
@droot droot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 15, 2019
@k8s-ci-robot k8s-ci-robot merged commit 87136d9 into kubernetes-sigs:master May 15, 2019
@DirectXMan12 DirectXMan12 deleted the feature/modules branch May 15, 2019 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants