Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

service catalog must check-in its vendored dependencies #620

Closed
derekwaynecarr opened this issue Mar 28, 2017 · 5 comments · Fixed by #639
Closed

service catalog must check-in its vendored dependencies #620

derekwaynecarr opened this issue Mar 28, 2017 · 5 comments · Fixed by #639
Milestone

Comments

@derekwaynecarr
Copy link
Contributor

derekwaynecarr commented Mar 28, 2017

The service catalog needs to check in its vendor code into the source tree.

  1. we cannot rely on vendored projects we rely on today existing tomorrow
  2. we need to be able to build past releases of service catalog in the future in case of security fixes, etc. not having those dependencies checked into the repository makes us unable to provide that guarantee
  3. we need to also ensure that every dependency must have a license per coding convention guidelines https://github.com/kubernetes/community/blob/42ec015a21824ab2698e1f7b9c8bf7bb4df69d49/contributors/devel/coding-conventions.md

While I cannot find it directly stated, I think this should be a requirement for incubation.

For reference on best practice, see examples below:

Kubernetes

https://github.com/kubernetes/kubernetes/tree/master/vendor
https://github.com/google/cadvisor/tree/master/vendor
https://github.com/kubernetes/kops/tree/master/vendor
https://github.com/kubernetes/heapster/tree/master/vendor
...

Kubernetes incubators
https://github.com/kubernetes-incubator/cri-o/tree/master/vendor
https://github.com/kubernetes-incubator/kube-aws/tree/master/vendor
https://github.com/kubernetes-incubator/kompose/tree/master/vendor
https://github.com/kubernetes-incubator/rktlet/tree/master/vendor
https://github.com/kubernetes-incubator/cluster-capacity/tree/master/vendor

@krancour
Copy link
Contributor

While I will not dispute the other points, to be fair, I have to correct this one:

all vendored code must have a version specified - existing glide.yaml does not make a version declaration for many items

glide.yaml does not; but glide.lock does-- for every dependency.

@pmorie
Copy link
Contributor

pmorie commented Mar 30, 2017

I agree that we must do this; I think that we should do it now.

@arschles
Copy link
Contributor

I don't like checking in vendor directories, but we need to do it. Dependencies are a golang-proper problem, and I don't want to waste any more time trying to solve it "elegantly".

@derekwaynecarr
Copy link
Contributor Author

@krancour - you are correct re: glide.lock. updated the original issue description.

@arschles arschles added this to the 0.0.2 milestone Apr 3, 2017
@MHBauer
Copy link
Contributor

MHBauer commented Apr 4, 2017

I am generally fed up with this aspect of go, so fine, do it, get it done, whatever I do not care anymore.

I do not see how linking to the vendor directory of other projects proves anything at all regarding best practices.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants