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

Move main.go to top-level. #27

Merged
merged 1 commit into from
May 23, 2017
Merged

Move main.go to top-level. #27

merged 1 commit into from
May 23, 2017

Conversation

jorinvo
Copy link
Contributor

@jorinvo jorinvo commented May 23, 2017

Makes fakedata go-gettable.

Or am I missing any reason here for having a cmd/ directory?
Doesn't seem like there will be other binaries in this repo.

@lucapette
Copy link
Owner

This breaks the make build command I think. I'm not sure what's the advantage of making it go gettable but maybe I'm missing something, can you please explain why that would help? I'm following the suggestions from this post which claims that the current structure makes the package go gettable so I'm even a bit confused now :)

Makes fakedata *go-gettable*.
@jorinvo
Copy link
Contributor Author

jorinvo commented May 23, 2017

Thanks for pointing out. I fixed the Makefile.

Many users like to install Go tools via go get. It works on any platform with Go install.

With the current setup this doesn't work:

$ go get github.com/lucapette/fakedata/
package github.com/lucapette/fakedata: no buildable Go source files in /Users/jorin/go/src/github.com/lucapette/fakedata

What still works is go get github.com/lucapette/fakedata/....

I also like the architecture Peter suggests in this article.
However, I think that having a cmd/ directory is only useful for projects that have more than one binary.
Since fakedata is single cli tool, there is no advantage in keeping cmd/.

@lucapette
Copy link
Owner

I have to admin I like more the structure with cmd even if there's a single binary in place but I don't like the fact the command then has to be go get github.com/lucapette/fakedata/... so I'm going to merge this despite my personal opinion :)

Thank you for taking care of it!

@lucapette lucapette merged commit c362510 into lucapette:master May 23, 2017
@jorinvo
Copy link
Contributor Author

jorinvo commented May 23, 2017

Thanks you 👍

@jorinvo jorinvo deleted the movemain branch May 23, 2017 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants