allegro / marathon-consul

Integrates Marathon apps with Consul service discovery.
Apache License 2.0
192 stars 33 forks source link

Add doc string for public methods #53

Open janisz opened 8 years ago

janisz commented 8 years ago

Use golint as a style check.

➜  marathon-consul git:(master) golint ./... 
main.go:14:5: exported var VERSION should have comment or be unexported
apps/app.go:8:6: exported type HealthCheck should have comment or be unexported
apps/app.go:18:6: exported type AppWrapper should have comment or be unexported
apps/app.go:22:6: exported type AppsResponse should have comment or be unexported
apps/app.go:22:6: type name will be used as apps.AppsResponse by other packages, and that stutters; consider calling this Response
apps/app.go:26:6: exported type App should have comment or be unexported
apps/app.go:33:1: comment on exported type AppId should be of the form "AppId ..." (with optional leading article)
apps/app.go:36:6: type AppId should be AppID
apps/app.go:42:1: exported method AppId.ConsulServiceName should have comment or be unexported
apps/app.go:46:1: exported method App.IsConsulApp should have comment or be unexported
apps/app.go:51:1: exported method App.ConsulServiceName should have comment or be unexported
apps/app.go:61:1: exported function ParseApps should have comment or be unexported
apps/app.go:68:1: exported function ParseApp should have comment or be unexported
apps/task.go:7:6: exported type Task should have comment or be unexported
apps/task.go:16:1: comment on exported type TaskId should be of the form "TaskId ..." (with optional leading article)
apps/task.go:18:6: type TaskId should be TaskID
apps/task.go:24:6: exported type HealthCheckResult should have comment or be unexported
apps/task.go:28:6: exported type TasksResponse should have comment or be unexported
apps/task.go:32:1: exported function ParseTasks should have comment or be unexported
apps/task.go:39:1: exported function ParseTask should have comment or be unexported
apps/task.go:45:1: exported method Task.IsHealthy should have comment or be unexported
config/config.go:17:6: exported type Config should have comment or be unexported
config/config.go:35:1: exported function New should have comment or be unexported
consul/agents.go:15:6: exported type Agents should have comment or be unexported
consul/agents.go:21:6: exported type ConcurrentAgents should have comment or be unexported
consul/agents.go:27:1: exported function NewAgents should have comment or be unexported
consul/agents.go:34:1: exported method ConcurrentAgents.GetAnyAgent should have comment or be unexported
consul/agents.go:45:28: method getRandomAgentIpAddress should be getRandomAgentIPAddress
consul/agents.go:47:17: should omit 2nd value from range; this loop is equivalent to `for ipAddress := range ...`
consul/agents.go:54:1: exported method ConcurrentAgents.RemoveAgent should have comment or be unexported
consul/agents.go:68:1: exported method ConcurrentAgents.GetAgent should have comment or be unexported
consul/config.go:5:6: exported type ConsulConfig should have comment or be unexported
consul/config.go:5:6: type name will be used as consul.ConsulConfig by other packages, and that stutters; consider calling this Config
consul/config.go:17:6: exported type Auth should have comment or be unexported
consul/consul.go:13:6: exported type ConsulServices should have comment or be unexported
consul/consul.go:13:6: type name will be used as consul.ConsulServices by other packages, and that stutters; consider calling this Services
consul/consul.go:17:13: interface method parameter serviceId should be serviceID
consul/consul.go:21:6: exported type Consul should have comment or be unexported
consul/consul.go:26:6: exported type ServicesProvider should have comment or be unexported
consul/consul.go:28:1: exported function New should have comment or be unexported
consul/consul.go:35:1: exported method Consul.GetServices should have comment or be unexported
consul/consul.go:42:45: should drop = nil from declaration of var services; it is the zero value
consul/consul.go:78:1: exported method Consul.GetAllServices should have comment or be unexported
consul/consul.go:119:1: exported method Consul.Register should have comment or be unexported
consul/consul.go:157:1: exported method Consul.Deregister should have comment or be unexported
consul/consul.go:157:29: method parameter serviceId should be serviceID
consul/consul.go:168:29: method parameter serviceId should be serviceID
consul/consul.go:183:1: exported method Consul.GetAgent should have comment or be unexported
consul/consul_stub.go:8:6: exported type ConsulStub should have comment or be unexported
consul/consul_stub.go:8:6: type name will be used as consul.ConsulStub by other packages, and that stutters; consider calling this Stub
consul/consul_stub.go:15:1: exported function NewConsulStub should have comment or be unexported
consul/consul_stub.go:19:1: exported function NewConsulStubWithTag should have comment or be unexported
consul/consul_stub.go:28:1: exported method ConsulStub.GetAllServices should have comment or be unexported
consul/consul_stub.go:43:1: exported method ConsulStub.GetServices should have comment or be unexported
consul/consul_stub.go:63:1: exported method ConsulStub.Register should have comment or be unexported
consul/consul_stub.go:66:9: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)
consul/consul_stub.go:76:1: exported method ConsulStub.Deregister should have comment or be unexported
consul/consul_stub.go:76:33: method parameter serviceId should be serviceID
consul/consul_stub.go:79:9: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)
consul/consul_stub.go:85:1: exported method ConsulStub.RegisteredServicesIds should have comment or be unexported
consul/consul_stub.go:94:1: exported method ConsulStub.GetAgent should have comment or be unexported
consul/consul_test_server.go:10:1: exported function CreateConsulTestServer should have comment or be unexported
consul/consul_test_server.go:16:1: exported function ConsulClientAtServer should have comment or be unexported
consul/consul_test_server.go:16:6: func name will be used as consul.ConsulClientAtServer by other packages, and that stutters; consider calling this ClientAtServer
events/deployment_info.go:9:6: exported type DeploymentEvent should have comment or be unexported
events/deployment_info.go:15:6: exported type Plan should have comment or be unexported
events/deployment_info.go:20:6: exported type Deployments should have comment or be unexported
events/deployment_info.go:21:2: struct field Id should be ID
events/deployment_info.go:26:6: exported type CurrentStep should have comment or be unexported
events/deployment_info.go:30:6: exported type Action should have comment or be unexported
events/deployment_info.go:32:2: struct field AppId should be AppID
events/deployment_info.go:76:1: exported method DeploymentEvent.StoppedConsulApps should have comment or be unexported
events/deployment_info.go:80:1: exported method DeploymentEvent.RestartedConsulApps should have comment or be unexported
events/deployment_info.go:84:1: exported method DeploymentEvent.RenamedConsulApps should have comment or be unexported
events/deployment_info.go:185:6: range var appId should be appID
events/deployment_info.go:185:13: should omit 2nd value from range; this loop is equivalent to `for appId := range ...`
events/deployment_info.go:193:1: exported function ParseDeploymentEvent should have comment or be unexported
events/events.go:10:2: exported var ErrNoEvent should have comment or be unexported
events/events.go:13:6: exported type Event should have comment or be unexported
events/events.go:18:6: exported type BaseEvent should have comment or be unexported
events/events.go:22:1: exported function EventType should have comment or be unexported
events/task_health_change.go:8:6: exported type TaskHealthChange should have comment or be unexported
events/task_health_change.go:17:1: exported function ParseTaskHealthChange should have comment or be unexported
marathon/config.go:5:6: exported type Config should have comment or be unexported
marathon/marathon.go:16:6: exported type Marathoner should have comment or be unexported
marathon/marathon.go:23:6: exported type Marathon should have comment or be unexported
marathon/marathon.go:30:6: exported type LeaderResponse should have comment or be unexported
marathon/marathon.go:34:1: exported function New should have comment or be unexported
marathon/marathon.go:58:1: exported method Marathon.App should have comment or be unexported
marathon/marathon.go:58:23: method parameter appId should be appID
marathon/marathon.go:69:1: exported method Marathon.Apps should have comment or be unexported
marathon/marathon.go:79:1: exported method Marathon.Tasks should have comment or be unexported
marathon/marathon.go:85:2: var trimmedAppId should be trimmedAppID
marathon/marathon.go:94:1: exported method Marathon.Leader should have comment or be unexported
marathon/marathon.go:142:17: should omit type string from declaration of var statusCode; it will be inferred from the right-hand side
marathon/marathon_stub.go:8:6: exported type MarathonerStub should have comment or be unexported
marathon/marathon_stub.go:15:1: exported method MarathonerStub.Apps should have comment or be unexported
marathon/marathon_stub.go:19:1: exported method MarathonerStub.App should have comment or be unexported
marathon/marathon_stub.go:22:9: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)
marathon/marathon_stub.go:27:1: exported method MarathonerStub.Tasks should have comment or be unexported
marathon/marathon_stub.go:27:31: method parameter appId should be appID
marathon/marathon_stub.go:30:9: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)
marathon/marathon_stub.go:35:1: exported method MarathonerStub.Leader should have comment or be unexported
marathon/marathon_stub.go:39:1: exported function MarathonerStubWithLeaderForApps should have comment or be unexported
marathon/marathon_stub.go:45:1: exported function MarathonerStubForApps should have comment or be unexported
metrics/config.go:5:6: exported type Config should have comment or be unexported
metrics/metrics.go:22:1: exported function Mark should have comment or be unexported
metrics/metrics.go:27:1: exported function Time should have comment or be unexported
metrics/metrics.go:32:1: exported function UpdateGauge should have comment or be unexported
metrics/metrics.go:37:1: exported function Init should have comment or be unexported
metrics/metrics.go:66:1: exported function TargetName should have comment or be unexported
sync/config.go:5:6: exported type Config should have comment or be unexported
sync/sync.go:15:6: exported type Sync should have comment or be unexported
sync/sync.go:21:1: exported function New should have comment or be unexported
sync/sync.go:25:1: exported method Sync.StartSyncServicesJob should have comment or be unexported
sync/sync.go:53:1: exported method Sync.SyncServices should have comment or be unexported
sync/sync.go:127:12: should omit 2nd value from range; this loop is equivalent to `for node := range ...`
sync/sync.go:138:3: var serviceId should be serviceID
sync/sync_test.go:157:49: method parameter instanceId should be instanceID
sync/sync_test.go:161:41: method parameter serviceId should be serviceID
sync/sync_test.go:364:30: method parameter appId should be appID
sync/sync_test.go:387:33: method parameter serviceId should be serviceID
utils/apps.go:8:1: exported function ConsulApp should have comment or be unexported
utils/apps.go:12:1: exported function ConsulAppWithUnhealthyInstances should have comment or be unexported
utils/apps.go:16:1: exported function NonConsulApp should have comment or be unexported
utils/net.go:8:1: exported function HostToIPv4 should have comment or be unexported
web/event_handler.go:16:6: exported type EventHandler should have comment or be unexported
web/event_handler.go:21:1: exported function NewEventHandler should have comment or be unexported
web/event_handler.go:28:1: exported method EventHandler.Handle should have comment or be unexported
web/event_handler.go:91:2: var appId should be appID
web/event_handler.go:130:6: func findTaskById should be findTaskByID
web/event_handler.go:130:35: don't use underscores in Go names; func parameter tasks_ should be tasks
web/event_handler.go:283:6: func replaceTaskIdWithId should be replaceTaskIDWithID
web/event_handler_test.go:824:37: func parameter taskId should be taskID
web/health_handler.go:8:1: exported function HealthHandler should have comment or be unexported
dankraw commented 8 years ago

Some (few) of the points are relevant, but imho adding that number of comments won't make our code any better. "Every time you write a comment, you should grimace and feel the failure of your ability of expression." I don't think we should blindly follow these rules, we're not a lib. :)

But if that tool can be configured to somehow filter/skip those "should have comment" warnings, that could be helpful though.

janisz commented 8 years ago

I agree but this should encourage us to double check if specific method/field really need to be public

wendigo commented 8 years ago

:+1: