Fixing static analysis issues

[edupo]

Running all the static analysis tools described here makes lots and lots of warnings and errors.

Some minor and some major (e.g.: cyclomatic complexity of some docker functions is very high, a big refactoring needs to be done.)

I'm not sure how to tackle this one, but I just add this as a reminder.

[lucymhdavies]

Current issues:

$ megacheck ./...
docker.go:221:20: os.Kill cannot be trapped (did you mean syscall.SIGTERM?) (SA1016)
docker.go:257:3: should check returned error before deferring hijack.Conn.Close() (SA5001)
docker.go:48:2: field running is unused (U1000)
The command "megacheck ./..." exited with 1.

Were easily fixed

0.01s$ gocyclo -over 19 $GO_FILES
22 cali (*DockerClient).StartContainer ./docker.go:191:1
The command "gocyclo -over 19 $GO_FILES" exited with 1.

^ like you say in https://github.com/skybet/cali/issues/18#issuecomment-378141923, this is gonna be a hard one to fix, but I'll take a look at what you've got on your fork.

Disabling this one for now.

We definitely need to refactor this function.

0.26s$ golint -set_exit_status $(go list ./...)
/home/travis/gopath/src/github.com/skybet/cali/cli.go:14:2: don't use ALL_CAPS in Go names; use CamelCase
/home/travis/gopath/src/github.com/skybet/cali/cli.go:14:2: exported const EXIT_CODE_RUNTIME_ERROR should have comment (or a comment on this block) or be unexported
/home/travis/gopath/src/github.com/skybet/cali/cli.go:15:2: don't use ALL_CAPS in Go names; use CamelCase
/home/travis/gopath/src/github.com/skybet/cali/command.go:11:1: comment on exported type Command should be of the form "Command ..." (with optional leading article)
/home/travis/gopath/src/github.com/skybet/cali/docker.go:24:2: struct field Id should be ID
/home/travis/gopath/src/github.com/skybet/cali/docker.go:36:2: struct field Id should be ID
/home/travis/gopath/src/github.com/skybet/cali/docker.go:51:1: comment on exported method DockerClient.InitDocker should be of the form "InitDocker ..."
/home/travis/gopath/src/github.com/skybet/cali/docker.go:112:1: comment on exported method DockerClient.AddEnv should be of the form "AddEnv ..."
/home/travis/gopath/src/github.com/skybet/cali/git.go:33:1: comment on exported method Git.Checkout should be of the form "Checkout ..."
/home/travis/gopath/src/github.com/skybet/cali/git.go:54:9: if block ends with a return statement, so drop this else and outdent its block
/home/travis/gopath/src/github.com/skybet/cali/git.go:90:1: exported method Git.Pull should have comment or be unexported
/home/travis/gopath/src/github.com/skybet/cali/git.go:143:6: func repoNameFromUrl should be repoNameFromURL
Found 12 lint suggestions; failing.
The command "golint -set_exit_status $(go list ./...)" exited with 1.

All fairly simple too.

[lucymhdavies]

Wasn't actually too hard to deal with the gocyclo issue afterall.

In https://github.com/skybet/cali/pull/46 I split out the code to run a container in non-interactive (and non-TTY) mode, into a method startContainerNonInteractive, which resulted in StartContainer becoming exactly 19 lines!

But then I had to add something to make it slightly longer.

I could do the same thing to split out another startContainerInteractive (and split out other parts of StartContainer too)