Complete lint for cmd/influx #4703
Conversation
|
Thanks @pablolmiranda -- you must sign the CLA before we can merge this. https://influxdb.com/community/cla.html |
| @@ -30,6 +30,7 @@ import ( | |||
| "github.com/influxdb/influxdb/services/udp" | |||
| "github.com/influxdb/influxdb/tcp" | |||
| "github.com/influxdb/influxdb/tsdb" | |||
| // What is the reason to have a blank import here? | |||
There was a problem hiding this comment.
This is to initialize the engine packages.
There was a problem hiding this comment.
So if lint requires this to be commented, please add the comment above.
|
@otoolep CLA signed. |
|
+1. |
|
Looks good. Needs a rebase to resolve the conflicts and then we can merge. Thanks! |
|
@otoolep @corylanou can you double check this PR? |
|
Looks good @pablolmiranda -- can you please squash your commits and I will then merge. A comment-only change like this should be a single commit. Also please send me an e-mail with your address so we can send you a shirt. |
|
Before you squash you should actually rebase your branch against our master.
|
| return fmt.Errorf("Failed to connect to %s\n", c.Client.Addr()) | ||
| } else { | ||
| c.ServerVersion = v |
There was a problem hiding this comment.
golint ask to move it out of the else block. But I removed by accident. Fixed now.
|
One question otherwise looks good. |
corrects the initialize package comment for lint brings c.ServerVersion back
pablolmiranda
force-pushed
the
lint-cmd-influx
branch
from
5f55426
to
b908cbb
Compare
Complete lint for cmd/influx
Part of #4098.