Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add doc string for public methods #53

Open
janisz opened this issue Feb 9, 2016 · 3 comments
Open

Add doc string for public methods #53

janisz opened this issue Feb 9, 2016 · 3 comments

Comments

@janisz
Copy link
Contributor

janisz commented Feb 9, 2016

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
Copy link
Contributor

dankraw commented Feb 9, 2016

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
Copy link
Contributor Author

janisz commented Feb 19, 2016

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

@wendigo
Copy link
Contributor

wendigo commented Mar 26, 2016

👍

@janisz janisz changed the title Add golint stylecheck Add doc string for public methods Aug 16, 2016
@janisz janisz added this to the 1.0 milestone Aug 16, 2016
@janisz janisz removed this from the 1.0 milestone Sep 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants