Conversation
| if err == nil { | ||
| dataUsage := strings.TrimSpace(string(output)) | ||
| if i, err := strconv.Atoi(dataUsage); err == nil { | ||
| if i, e := strconv.Atoi(dataUsage); e == nil { |
There was a problem hiding this comment.
The linter was giving warnings about the second err shadowing the one from it's parent scope.
| return runtime.GOOS == "linux" | ||
| } | ||
|
|
||
| // IsMac detects if we are running on the darwin platform |
There was a problem hiding this comment.
Shouldn't we have IsLinux?
Making the other checks more feature driven seems a fine followup
There was a problem hiding this comment.
Are you sugesting an IsLinux in addition to SupportsNativeDocker()? Most of the linux checks are for a feature (native docker support) where as some of the mac checks were for things like which route command to use or this process to kill to restart dns resolution. So there is a bit of the mix in platform name and feature detection. This is the quicked line to remove linter warnings and also make the code more readable (and centralized if for some crazy reason the string "darwin" goes away.
There was a problem hiding this comment.
My thinking was for API completeness if we have checks for the OS's we may as well have one for each that we support.
|
Did I miss travis config? |
|
I didn;t add travis configuration yet. I was not sure if we wanted to fail the build on linter warnings. Maybe we should? I had to exempt 3 functions from linting, one due to some nested error conditions that was not ready mentally to unpack and 2 for cyclomatic complexity. They should all be reworked but I wanted to get the superficial warnings covered and then take an separate PR to attack the complexity issues in Doctor and ProjectConfig validation. |
|
Added travis integration for the linter |
grayside
left a comment
grayside
left a comment
There was a problem hiding this comment.
To answer the last question, my take on it is that we add it to Travis so that gometalinter's recommendations are forced into how rig is built, and therefore into how we write go code. We can choose to exempt code on a case by case basis if it's necessary.
Let's also file a follow-up issue for each TODO.
3 participants
added linting and addressed all feedback (and ignored 3)
Addresses #92