search

infobloxopen / infoblox-go-client

Infoblox NIOS WAPI Go Client library
Apache License 2.0
35 stars 88 forks source link

Make golint happy #41

Open eest opened 6 years ago

eest commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Agreed we should make go-lint happy.

yuewko commented 6 years ago

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 commented 6 years ago

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.