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

Make golint happy #41

Open
eest opened this issue Mar 1, 2018 · 5 comments
Open

Make golint happy #41

eest opened this issue Mar 1, 2018 · 5 comments

Comments

@eest
Copy link
Contributor

eest commented Mar 1, 2018

I noticed that running golint (https://github.com/golang/lint) will output a bunch of suggestions against master. Most of them are related to missing documentation comments for exported functions, but also some letter case fixes and code construct suggestions. I am opening an issue instead of a PR because it is unclear to me how these things should be handled and I am not in a good position to document the package.

Output:

connector.go:20:6: exported type HostConfig should have comment or be unexported
connector.go:28:6: exported type TransportConfig should have comment or be unexported
connector.go:31:2: struct field HttpRequestTimeout should be HTTPRequestTimeout
connector.go:32:2: struct field HttpPoolConnections should be HTTPPoolConnections
connector.go:35:1: exported function NewTransportConfig should have comment or be unexported
connector.go:49:10: should replace errors.New(fmt.Sprintf(...)) with fmt.Errorf(...)
connector.go:61:6: exported type HttpRequestBuilder should have comment or be unexported
connector.go:61:6: type HttpRequestBuilder should be HTTPRequestBuilder
connector.go:68:6: exported type HttpRequestor should have comment or be unexported
connector.go:68:6: type HttpRequestor should be HTTPRequestor
connector.go:73:6: exported type WapiRequestBuilder should have comment or be unexported
connector.go:77:6: exported type WapiHttpRequestor should have comment or be unexported
connector.go:77:6: type WapiHttpRequestor should be WapiHTTPRequestor
connector.go:81:6: exported type IBConnector should have comment or be unexported
connector.go:88:6: exported type Connector should have comment or be unexported
connector.go:95:6: exported type RequestType should have comment or be unexported
connector.go:98:2: exported const CREATE should have comment (or a comment on this block) or be unexported
connector.go:119:6: func getHttpResponseError should be getHTTPResponseError
connector.go:127:1: exported method WapiHttpRequestor.Init should have comment or be unexported
connector.go:144:1: exported method WapiHttpRequestor.SendRequest should have comment or be unexported
connector.go:165:1: exported method WapiRequestBuilder.Init should have comment or be unexported
connector.go:169:1: exported method WapiRequestBuilder.BuildUrl should have comment or be unexported
connector.go:169:32: method BuildUrl should be BuildURL
connector.go:196:1: exported method WapiRequestBuilder.BuildBody should have comment or be unexported
connector.go:197:6: var objJson should be objJSON
connector.go:208:3: var eaSearchJson should be eaSearchJSON
connector.go:219:1: exported method WapiRequestBuilder.BuildRequest should have comment or be unexported
connector.go:254:1: exported method Connector.CreateObject should have comment or be unexported
connector.go:272:1: exported method Connector.GetObject should have comment or be unexported
connector.go:292:1: exported method Connector.DeleteObject should have comment or be unexported
connector.go:310:1: exported method Connector.UpdateObject should have comment or be unexported
connector.go:327:1: exported method Connector.Logout should have comment or be unexported
connector.go:336:5: exported var ValidateConnector should have comment or be unexported
connector.go:349:1: exported function NewConnector should have comment or be unexported
connector_test.go:30:31: method BuildUrl should be BuildURL
connector_test.go:50:6: type FakeHttpRequestor should be FakeHTTPRequestor
connector_test.go:94:5: var expectedUrlStr should be expectedURLStr
connector_test.go:106:5: var expectedUrlStr should be expectedURLStr
connector_test.go:117:5: var expectedUrlStr should be expectedURLStr
lock.go:15:6: exported type Lock should have comment or be unexported
lock.go:20:6: exported type NetworkViewLock should have comment or be unexported
lock.go:185:1: exported method NetworkViewLock.Lock should have comment or be unexported
lock.go:210:10: if block ends with a return statement, so drop this else and outdent its block
lock.go:225:1: exported method NetworkViewLock.UnLock should have comment or be unexported
object_manager.go:10:6: exported type IBObjectManager should have comment or be unexported
object_manager.go:29:6: exported type ObjectManager should have comment or be unexported
object_manager.go:35:1: exported function NewObjectManager should have comment or be unexported
object_manager.go:45:41: method parameter cloudApiOwned should be cloudAPIOwned
object_manager.go:53:1: exported method ObjectManager.CreateNetworkView should have comment or be unexported
object_manager.go:80:1: exported method ObjectManager.CreateDefaultNetviews should have comment or be unexported
object_manager.go:92:1: exported method ObjectManager.CreateNetwork should have comment or be unexported
object_manager.go:110:1: exported method ObjectManager.CreateNetworkContainer should have comment or be unexported
object_manager.go:122:1: exported method ObjectManager.GetNetworkView should have comment or be unexported
object_manager.go:136:1: exported method ObjectManager.UpdateNetworkViewEA should have comment or be unexported
object_manager.go:151:9: should omit 2nd value from range; this loop is equivalent to `for k := range ...`
object_manager.go:162:1: exported function BuildNetworkViewFromRef should have comment or be unexported
object_manager.go:177:1: exported function BuildNetworkFromRef should have comment or be unexported
object_manager.go:193:1: exported method ObjectManager.GetNetwork should have comment or be unexported
object_manager.go:216:1: exported method ObjectManager.GetNetworkContainer should have comment or be unexported
object_manager.go:232:1: exported function GetIPAddressFromRef should have comment or be unexported
object_manager.go:243:1: exported method ObjectManager.AllocateIP should have comment or be unexported
object_manager.go:273:1: exported method ObjectManager.AllocateNetwork should have comment or be unexported
object_manager.go:292:1: exported method ObjectManager.GetFixedAddress should have comment or be unexported
object_manager.go:313:1: exported method ObjectManager.UpdateFixedAddress should have comment or be unexported
object_manager.go:332:1: exported method ObjectManager.ReleaseIP should have comment or be unexported
object_manager.go:340:1: exported method ObjectManager.DeleteNetwork should have comment or be unexported
object_manager.go:349:1: exported method ObjectManager.GetEADefinition should have comment or be unexported
object_manager.go:363:1: exported method ObjectManager.CreateEADefinition should have comment or be unexported
object_manager.go:437:1: comment on exported method ObjectManager.GetGridLicense should be of the form "GetGridLicense ..."
objects.go:9:7: don't use ALL_CAPS in Go names; use CamelCase
objects.go:9:7: exported const MACADDR_ZERO should have comment or be unexported
objects.go:11:6: exported type Bool should have comment or be unexported
objects.go:13:6: exported type EA should have comment or be unexported
objects.go:15:6: exported type EASearch should have comment or be unexported
objects.go:17:6: exported type EADefListValue should have comment or be unexported
objects.go:19:6: exported type IBBase should have comment or be unexported
objects.go:25:6: exported type IBObject should have comment or be unexported
objects.go:32:1: exported method IBBase.ObjectType should have comment or be unexported
objects.go:36:1: exported method IBBase.ReturnFields should have comment or be unexported
objects.go:40:1: exported method IBBase.EaSearch should have comment or be unexported
objects.go:44:6: exported type NetworkView should have comment or be unexported
objects.go:51:1: exported function NewNetworkView should have comment or be unexported
objects.go:68:1: exported function NewUpgradeStatus should have comment or be unexported
objects.go:90:6: exported type Network should have comment or be unexported
objects.go:98:1: exported function NewNetwork should have comment or be unexported
objects.go:106:6: exported type ServiceStatus should have comment or be unexported
objects.go:112:6: exported type LanHaPortSetting should have comment or be unexported
objects.go:120:6: exported type PhysicalPortSetting should have comment or be unexported
objects.go:126:6: exported type NetworkSetting should have comment or be unexported
objects.go:133:2: struct field VlanId should be VlanID
objects.go:135:6: exported type Ipv6Setting should have comment or be unexported
objects.go:142:2: struct field VirtualIp should be VirtualIP
objects.go:143:2: struct field VlanId should be VlanID
objects.go:147:6: exported type NodeInfo should have comment or be unexported
objects.go:149:2: struct field HwId should be HwID
objects.go:175:1: exported function NewMember should have comment or be unexported
objects.go:197:1: exported function NewGridLicense should have comment or be unexported
objects.go:210:1: exported function NewLicense should have comment or be unexported
objects.go:239:1: exported function NewCapcityReport should have comment or be unexported
objects.go:247:6: exported type NTPserver should have comment or be unexported
objects.go:256:6: exported type NTPSetting should have comment or be unexported
objects.go:264:6: exported type Grid should have comment or be unexported
objects.go:271:1: exported function NewGrid should have comment or be unexported
objects.go:279:6: exported type NetworkContainer should have comment or be unexported
objects.go:287:1: exported function NewNetworkContainer should have comment or be unexported
objects.go:295:6: exported type FixedAddress should have comment or be unexported
objects.go:305:1: exported function NewFixedAddress should have comment or be unexported
objects.go:313:6: exported type EADefinition should have comment or be unexported
objects.go:324:1: exported function NewEADefinition should have comment or be unexported
objects.go:332:6: exported type UserProfile should have comment or be unexported
objects.go:338:1: exported function NewUserProfile should have comment or be unexported
objects.go:346:6: exported type RecordA should have comment or be unexported
objects.go:356:1: exported function NewRecordA should have comment or be unexported
objects.go:364:6: exported type RecordCNAME should have comment or be unexported
objects.go:374:1: exported function NewRecordCNAME should have comment or be unexported
objects.go:382:6: exported type RecordHostIpv4Addr should have comment or be unexported
objects.go:386:6: exported type RecordHost should have comment or be unexported
objects.go:397:1: exported function NewRecordHost should have comment or be unexported
objects.go:405:6: exported type RecordTXT should have comment or be unexported
objects.go:415:1: exported function NewRecordTXT should have comment or be unexported
objects.go:423:6: exported type ZoneAuth should have comment or be unexported
objects.go:431:1: exported function NewZoneAuth should have comment or be unexported
objects.go:439:1: exported method EA.MarshalJSON should have comment or be unexported
objects.go:450:1: exported method EASearch.MarshalJSON should have comment or be unexported
objects.go:459:1: exported method EADefListValue.MarshalJSON should have comment or be unexported
objects.go:466:1: exported method Bool.MarshalJSON should have comment or be unexported
objects.go:474:1: exported method EA.UnmarshalJSON should have comment or be unexported
objects.go:503:1: exported method EADefListValue.UnmarshalJSON should have comment or be unexported
objects.go:503:1: receiver name v should be consistent with previous receiver name val for EADefListValue
objects.go:514:6: exported type RequestBody should have comment or be unexported
objects.go:524:6: exported type SingleRequest should have comment or be unexported
objects.go:529:6: exported type MultiRequest should have comment or be unexported
objects.go:534:1: exported method MultiRequest.MarshalJSON should have comment or be unexported
objects.go:538:1: exported function NewMultiRequest should have comment or be unexported
objects.go:544:1: exported function NewRequest should have comment or be unexported
@chinmayb
Copy link
Contributor

chinmayb commented Mar 1, 2018

I feel lint errors like "exported method NetworkViewLock.Lock should have comment or be unexported" are bit silly. Many of the times methods/functions are self explanatory. Adding comments dont really add any value at times. just my view.

@eest
Copy link
Contributor Author

eest commented Mar 1, 2018

I understand what you mean. I bring it up because I feel it is helpful to have golint in my pre-commit checklist in order to more easily catch code that does not look like idiomatic Go before sending patches.

@johnbelamaric
Copy link
Contributor

Agreed we should make go-lint happy.

@yuewko
Copy link
Contributor

yuewko commented Mar 1, 2018

Agreed also!

Additional reasons:

  1. Sometimes even it might look self explanatory to the coder, there is typically details that might not be so obvious for a new comer to the code - these details should be included in those comments.

  2. Note that godoc uses those comments to generate documentation.

  3. Consistency is a good thing.

@eest
Copy link
Contributor Author

eest commented Mar 2, 2018

I have gone through the golint output and opened PRs for all the internal stuff that can be modified without thinking about external dependencies. I have also documented the external Logout method that I was responsible for adding.

I do not plan on touching any of the exported functions/methods/struct fields since this breaks the current contract with library users and therefore requires a higher level of coordination. I also think that documentation for exported functions is better handled by the function authors instead of me doing more or less qualified guesswork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants