carbocation / go-instagram

Go library for accessing Instagram REST and Search APIs
BSD 2-Clause "Simplified" License
51 stars 20 forks source link

Updating project structure #2

Closed BrandonRomano closed 9 years ago

BrandonRomano commented 9 years ago

Updating to the proper structure for a library.

Tools such as godep won't be able to understand the current structure.

carbocation commented 9 years ago

This would break any existing project, as the current import path is github.com/carbocation/go-instagram/instagram but this would change the import path to github.com/carbocation/go-instagram, unless I am misunderstanding something. I don't currently use godep (though I likely will if the community standardizes on it), but can you help me understand what the issue is with the current structure?

BrandonRomano commented 9 years ago

So, you're right -- this would break any existing project structure. Likely some type of versioning would be required.

So the problem isn't really only with godep, but with go get (which godep uses) which is something the community has agreed on.

Look at this when we run a go get on the package with this command:

go get github.com/carbocation/go-instagram

We get get this error message:

package github.com/carbocation/go-instagram
    imports github.com/carbocation/go-instagram
    imports github.com/carbocation/go-instagram: no buildable Go source files in /$GOPATH/src/github.com/carbocation/go-instagram

The issue is that the Github URL that actually hosts your package is not the same as your package's URL in go code. go get is actually dependent on this consistency.

go get is supposed to be responsible for fetching + building -- where as the current structure will only fetch and the user will end up having to manually build the package.

carbocation commented 9 years ago

I didn't choose the initial package layout (this is a fork) but the actual package is at sub folder /instagram

If I recall correctly, hyphens are frowned upon in official package names. I would propose to put the examples within the instagram sub folder rather than moving everything into the root. Would that be of use?

On Monday, December 8, 2014, Brandon notifications@github.com wrote:

So, you're right -- this would break any existing project structure. Likely some type of versioning would be required.

So the problem isn't really only with godep, but with go get (which godep uses) which is something the community has agreed on.

Look at this when we run a go get on the package with this command:

go get github.com/carbocation/go-instagram

We get get this error message:

package github.com/carbocation/go-instagram imports github.com/carbocation/go-instagram imports github.com/carbocation/go-instagram: no buildable Go source files in /$GOPATH/src/github.com/carbocation/go-instagram

The issue is that the Github URL that actually hosts your package is not the same as your package's URL in go code. go get is actually dependent on this consistency.

go get is supposed to be responsible for fetching + building -- where as the current structure will only fetch and the user will end up having to manually build the package.

— Reply to this email directly or view it on GitHub https://github.com/carbocation/go-instagram/pull/2#issuecomment-66123348 .

BrandonRomano commented 9 years ago

So that really wouldn't be of any use, as the git URL would still have to point to that package for go get to actually compile.

So in this specific case, as your git url is github.com/carbocation/go-instagram, so the actual root folder of the library would have to be there.

carbocation commented 9 years ago

Surely the tool isn't so limited that it can't point to a sub folder?

On Monday, December 8, 2014, Brandon notifications@github.com wrote:

So that really wouldn't be of any use, as the git URL would still have to point to that package for go get to actually compile.

So in this specific case, as your git url is github.com/carbocation/go-instagram, so the actual root folder of the library would have to be there.

— Reply to this email directly or view it on GitHub https://github.com/carbocation/go-instagram/pull/2#issuecomment-66128404 .

BrandonRomano commented 9 years ago

To my understanding you can't. I think that was an intentional decision by the creators of go.

https://golang.org/cmd/go/#hdr-Download_and_install_packages_and_dependencies

It does seem like a shortcoming though, and I don't disagree with you.

carbocation commented 9 years ago

I use the library as currently structured without issues. Can you construct a project such that it is broken with the current structure to demonstrate that this is an actual problem rather than a theoretical one?

On Monday, December 8, 2014, Brandon notifications@github.com wrote:

To my understanding you can't. I think that was an intentional decision by the creators of go.

https://golang.org/cmd/go/#hdr-Download_and_install_packages_and_dependencies

It does seem like a shortcoming though, and I don't disagree with you.

— Reply to this email directly or view it on GitHub https://github.com/carbocation/go-instagram/pull/2#issuecomment-66134255 .

carbocation commented 9 years ago

Sorry if I sound terse; rounding in the MICU as I write this so not much time for nicer phrasing.

On Monday, December 8, 2014, James Pirruccello < james.pirruccello@aya.yale.edu> wrote:

I use the library as currently structured without issues. Can you construct a project such that it is broken with the current structure to demonstrate that this is an actual problem rather than a theoretical one?

On Monday, December 8, 2014, Brandon <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

To my understanding you can't. I think that was an intentional decision by the creators of go.

https://golang.org/cmd/go/#hdr-Download_and_install_packages_and_dependencies

It does seem like a shortcoming though, and I don't disagree with you.

— Reply to this email directly or view it on GitHub https://github.com/carbocation/go-instagram/pull/2#issuecomment-66134255 .

BrandonRomano commented 9 years ago

So you can still USE this library, you just have to manually compile it yourself. go get will not compile it, which is 1/2 of it's intended functionality.

The issue actually came to me when I tried to use kr's heroku buildpack -- which is dependent on actually using go get and having it install. If it doesn't install with go get, it never installs which makes it fail to deploy.

Because this project is not 100% compatible with go get, you can't use this project using the buildpack.

This problem is not theoretical, I'm just describing it from a high level because I don't want to localize this issue to my specific issue, as go get not working as intended can have other implications.

This conversations dragging on a little longer than I thought it was going to, so I'll catch up with you later if you'd like to have further discussion -- I'm at work right now.

carbocation commented 9 years ago

I will try to reproduce this locally. Maybe it's always worked for me because I had the repo on my computer for development! Thanks for the heads up, I should be able to reproduce this easily.

On Monday, December 8, 2014, Brandon notifications@github.com wrote:

So you can still USE this library, you just have to manually compile it yourself. go get will not compile it, which is 1/2 of it's intended functionality.

The issue actually came to me when I tried to use kr's heroku buildpack https://github.com/kr/heroku-buildpack-go -- which is dependent on actually using go get and having it install. If it doesn't install with go get, it never installs which makes it fail to deploy.

Because this project is not 100% compatible with go get, you can't use this project using the buildpack.

This problem is not theoretical, I'm just describing it from a high level because I don't want to localize this issue to my specific issue, as go get not working as intended can have other implications.

This conversations dragging on a little longer than I thought it was going to, so I'll catch up with you later if you'd like to have further discussion -- I'm at work right now.

— Reply to this email directly or view it on GitHub https://github.com/carbocation/go-instagram/pull/2#issuecomment-66138054 .

carbocation commented 9 years ago

The stated issue is that go get will not work with the library as it is currently structured. But when I delete the package locally and then go get "github.com/carbocation/go-instagram/instagram", it works without issue.

carbocation commented 9 years ago

I'm going to close this because I cannot reproduce the issue that this is designed to solve.