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

Make go get'able (or at least go installable) #216

Closed
ericchiang opened this issue Oct 18, 2016 · 6 comments
Closed

Make go get'able (or at least go installable) #216

ericchiang opened this issue Oct 18, 2016 · 6 comments
Assignees

Comments

@ericchiang
Copy link

ericchiang commented Oct 18, 2016

It's odd that kompose's main package is named main rather than kompose like most Go projects Additionally, since there's no top level package it seems like it'd be reasonable to enable go get github.com/kubernetes-incubator/kompose to work.

Additionally there's a dependence on -tags experimental due to a docker dependency[0]. Removing that import here[1], (say by copying the structs) would also break the dependency on github.com/docker/docker and remove 19 direct imports from vendor.

$ cat Godeps/Godeps.json | jq .Deps[].ImportPath | grep '"github.com/docker/docker'
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/blkiodev"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/filters"
"github.com/docker/docker/api/types/mount"
"github.com/docker/docker/api/types/network"
"github.com/docker/docker/api/types/registry"
"github.com/docker/docker/api/types/strslice"
"github.com/docker/docker/api/types/swarm"
"github.com/docker/docker/api/types/versions"
"github.com/docker/docker/cli/command/bundlefile"
"github.com/docker/docker/opts"
"github.com/docker/docker/pkg/mount"
"github.com/docker/docker/pkg/signal"
"github.com/docker/docker/pkg/system"
"github.com/docker/docker/pkg/term"
"github.com/docker/docker/pkg/term/windows"
"github.com/docker/docker/pkg/urlutil"
"github.com/docker/docker/runconfig/opts"

[0] https://github.com/docker/docker/blob/15ea28f6db7339f8a44078f3ef70484c96eebce7/cli/command/bundlefile/bundlefile.go
[1] https://github.com/kubernetes-incubator/kompose/blob/d9899b788d0e2051221e1b993f5a48268f9f3758/pkg/loader/bundle/bundle.go#L26

@sebgoa
Copy link
Contributor

sebgoa commented Oct 20, 2016

@ngtuna can you take this ?

@ngtuna ngtuna self-assigned this Oct 20, 2016
@ngtuna
Copy link
Contributor

ngtuna commented Oct 20, 2016

@ericchiang I added package.go at #227 for making kompose go get-able. IMHO main package named main is okay. There are a lot of Go projects doing like that.

Yes removing -tags experimental is a good point. However it's not only copy structs but kompose also depends on bundlefile's loadFile I am afraid that will need a "big" effort to completely remove importing bundlefile at this moment.

@ericchiang
Copy link
Author

@ngtuna

There are a lot of Go projects doing like that.

Sure, but Kubernetes isn't one of those projects. And almost all other kubernetes-incubator repos use the cmd/( binary name ) structure used by the Go project (see: https://godoc.org/golang.org/x/tools/cmd)

However it's not only copy structs but kompose also depends on bundlefile's loadFile I am afraid that will need a "big" effort to completely remove importing bundlefile at this moment.

LoadFile is just calling json.Decoder(r).Decode(bundle).

https://github.com/docker/docker/blob/2c620d0aa24c5f774a9115449a86b158b005bba8/cli/command/bundlefile/bundlefile.go#L37-L59

@ngtuna
Copy link
Contributor

ngtuna commented Oct 20, 2016

LoadFile is just calling json.Decoder(r).Decode(bundle).

Oh you're right... My mistake didn't take a look into the function's detail. The thing is not simple like that with compose file parsing function.

This was referenced Oct 20, 2016
@cdrage
Copy link
Member

cdrage commented Oct 25, 2016

@ngtuna I'm assuming it'll get rid of this error when you go get then?

▶ go get github.com/kubernetes-incubator/kompose
package github.com/kubernetes-incubator/kompose: no buildable Go source files in /home/wikus/dropbox/dev/go/src/github.com/kubernetes-incubator/kompose

@ngtuna
Copy link
Contributor

ngtuna commented Oct 25, 2016

@cdrage at least it will yes, and we expect to get the binary as well.

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

No branches or pull requests

4 participants