Skip to content
This repository has been archived by the owner on Aug 14, 2020. It is now read-only.

remove k8s.io/kubernetes dependency #684

Merged
merged 1 commit into from
Jan 26, 2017
Merged

remove k8s.io/kubernetes dependency #684

merged 1 commit into from
Jan 26, 2017

Conversation

cgonyeo
Copy link
Member

@cgonyeo cgonyeo commented Jan 26, 2017

This commit removes k8s.io/kubernetes as a dependency. The one part of
the repo that we were using, k8s.io/kubernetes/pkg/api/resource, has
been copied to schema/types/resource, and very minimally modified to
work in its new location. A README.md has been added to this new
location describing where the files came from.

This was necessary to avoid a cyclic dependency with the kubernetes
repo.

Fixes #683.

@cgonyeo
Copy link
Member Author

cgonyeo commented Jan 26, 2017

One thing about this I was uncertain about was that I left all of the copyright attributions at the top of the copied in files as Copyright 2014 The Kubernetes Authors, should this be changed?

@s-urbaniak
Copy link
Contributor

woah, thanks!

I guess we can cut a bit more, namely the whole protobuf dependency.

Looking at what this type implements now it seems a bit too much:

 struct type Quantity
    implements encoding/json.Marshaler
 pointer type *Quantity
    implements encoding/json.Unmarshaler
    implements fmt.Stringer
    implements github.com/appc/docker2aci/vendor/github.com/gogo/protobuf/proto.Marshaler
    implements github.com/appc/docker2aci/vendor/github.com/gogo/protobuf/proto.Message
    implements github.com/appc/docker2aci/vendor/github.com/gogo/protobuf/proto.Sizer
    implements github.com/appc/docker2aci/vendor/github.com/gogo/protobuf/proto.Unmarshaler
    implements github.com/appc/spec/vendor/github.com/gogo/protobuf/proto.Marshaler
    implements github.com/appc/spec/vendor/github.com/gogo/protobuf/proto.Message
    implements github.com/appc/spec/vendor/github.com/gogo/protobuf/proto.Sizer
    implements github.com/appc/spec/vendor/github.com/gogo/protobuf/proto.Unmarshaler
    implements github.com/golang/protobuf/proto.Marshaler
    implements github.com/golang/protobuf/proto.Message
    implements github.com/golang/protobuf/proto.Unmarshaler
    implements k8s.io/kubernetes/vendor/github.com/gogo/protobuf/proto.Marshaler
    implements k8s.io/kubernetes/vendor/github.com/gogo/protobuf/proto.Message
    implements k8s.io/kubernetes/vendor/github.com/gogo/protobuf/proto.Sizer
    implements k8s.io/kubernetes/vendor/github.com/gogo/protobuf/proto.Unmarshaler
    implements runtime.stringer

Some of the above are redundant, reducing to:

    implements k8s.io/kubernetes/vendor/github.com/gogo/protobuf/proto.Marshaler
    implements k8s.io/kubernetes/vendor/github.com/gogo/protobuf/proto.Message
    implements k8s.io/kubernetes/vendor/github.com/gogo/protobuf/proto.Sizer
    implements k8s.io/kubernetes/vendor/github.com/gogo/protobuf/proto.Unmarshaler

which all reside in quantity_proto.go. I suggest we delete this file, and thus also get rid of the protobuf dependency. Also I suggest we delete the generated protobuf. Summarizing I would also get rid of those files:

  • schema/types/resource/generated.pb.go
  • schema/types/resource/generated.proto
  • schema/types/resource/quantity_proto.go

This Quantity type is quite involved, using fixed point arithmetic, so I would leave all those tests.

This commit removes k8s.io/kubernetes as a dependency. The one part of
the repo that we were using, k8s.io/kubernetes/pkg/api/resource, has
been copied to schema/types/resource, and very minimally modified to
work in its new location. A README.md has been added to this new
location describing where the files came from.

This was necessary to avoid a cyclic dependency with the kubernetes
repo.
@cgonyeo
Copy link
Member Author

cgonyeo commented Jan 26, 2017

Removed the files you requested

Copy link
Contributor

@s-urbaniak s-urbaniak left a comment

Choose a reason for hiding this comment

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

LGTM

@cgonyeo cgonyeo merged commit fc380db into appc:master Jan 26, 2017
@sttts
Copy link

sttts commented Jan 26, 2017

Thanks for the fast solution of the issue! 👍

@cgonyeo
Copy link
Member Author

cgonyeo commented Jan 26, 2017

No problem! Now we should just need a new release to use on the k8s side of this. I'll leave that to someone else to take care of...

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 this pull request may close these issues.

3 participants