Promote cmd out of internal

[amckinney]

In response to #192, this moves the ./internal/cmd package to ./cmd so that it more clearly expresses the project's usability with go get when we introduce a go.mod file.

Note that this also allows projects to effectively version control prototool.
For example, in dependency management systems like glide, we can add the following lines to the glide.yaml:

- package: github.com/uber/prototool
  version: v1.2.0

However, glide will not pick up the transitive dependencies for prototool by default unless the package is actually depended on. Now that cmd is out of internal/, we can add a simple import like so:

import _ "github.com/uber/prototool/cmd"

[codecov-io]

Codecov Report

Merging #217 into dev will decrease coverage by 5.59%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              dev     #217     +/-   ##
=========================================
- Coverage   68.84%   63.24%   -5.6%     
=========================================
  Files          73       74      +1     
  Lines        3733     4201    +468     
=========================================
+ Hits         2570     2657     +87     
- Misses        840     1207    +367     
- Partials      323      337     +14

Impacted Files	Coverage Δ
cmd/flags.go	100% <ø> (ø)
internal/exec/exec.go	68.42% <ø> (ø)	⬆️
cmd/cmd.go	67.16% <ø> (ø)
cmd/internal/testdata/grpc/gen/grpcpb/grpc.pb.go	19.79% <ø> (ø)
cmd/templates.go	80.64% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26b9512...38a3800. Read the comment docs.

[peats-bond]

### Changed

[amckinney]

Good catch - using Added since this is more additive from a user perspective. Changed is a change in behavior and sometimes implies a breaking change.

[peats-bond]

missing new lines at the end of these proto files

[peats-bond]

Second thought, why are we introducing these files?

[amckinney]

Oof, I forgot to update the .gitignore in my project-wide find/replace. Fixed 👍

[peats-bond]

super nitty:

Move `internal/cmd` to `cmd/` so that `prototool` can be imported by other projects.

[bufdev]

I think I'm missing something - this PR doesn't change that prototool can not be used properly with go get. By doing this, we're allowing what we did not want to allow. Additionally, the commands gen-prototool-bash-completion, gen-prototool-zsh-completion, gen-prototool-manpages should never be public, these are just helpers for the release.

I'm probably missing something though, should we talk offline?

[amckinney]

@peter-edge Sorry, the description wasn't clear - you're not missing anything. We still can't use go get effectively until we have the vgo integration. The description would have been better phrased as:

In response to #192, this moves the ./internal/cmd package to ./cmd so that it more clearly expresses the project's usability with go get when we introduce a go.mod file.

I was attempting to solve the version control issue (i.e. the glide example) and saw this as "killing two birds with one stone" since it would perform part of what is required in #192. Seeing as this doesn't really change the behavior of go get at all, I thought it would better belong in a separate change.

What are your thoughts? Next steps would be to include a go.mod file (potentially in a separate PR) and move the gen* commands back into internal packages (in this PR).

[bufdev]

My general thoughts:

• The only thing that should be made external eventually is cmd/prototool, not cmd as a whole (unless depending on cmd as a library is needed, but I think you'd want to expose individual parts, see Promote Lint out of internal + Backwards compatibility checker #219), and for sure not the helper release commands :-)
• If you're going to make it external, you should do it only once you have the go.mod file in place, so that go get works as appropriate with Go 1.11 (and update the documentation to reflect that go get only works with Go 1.11+ as well, and only if GO111MODULE=on is set I believe). If you do this change as-is, it is an indicator that go get works right now, which would not be the case as the PR is currently written. That will mean more filed issues/installation confusion for end-users IMO.

[bufdev]

Also note I started doing the conversion in #197, however there's a bunch of issues to doing this properly, most notably that you will want a build tag for testing dependencies, and especially for tool dependencies. See golang/go#24661 to get started. Also see golang/go#25922.

[amckinney]

Sweet, that sounds reasonable. Thanks for the links! Let's give this a look and make sure everything is in place before we move this forward. I'm happy to defer / close this for now and re-open when it is relevant.