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

Fix golint warnings #422

Merged
merged 6 commits into from
Mar 12, 2019
Merged

Fix golint warnings #422

merged 6 commits into from
Mar 12, 2019

Conversation

ilammy
Copy link
Collaborator

@ilammy ilammy commented Mar 11, 2019

I've installed the latest golint and walked through the warnings. This should improve out future Go Report, but I'm not sure that we'll get 90%+ score. There are still some warnings that we can't fix without breaking backwards compatibility by renaming constants, structures and methods:

cell/cell.go:135:2: don't use ALL_CAPS in Go names; use CamelCase
cell/cell.go:136:2: don't use ALL_CAPS in Go names; use CamelCase
cell/cell.go:137:2: don't use ALL_CAPS in Go names; use CamelCase
compare/compare.go:67:2: don't use ALL_CAPS in Go names; use CamelCase
compare/compare.go:68:2: don't use ALL_CAPS in Go names; use CamelCase
compare/compare.go:69:2: don't use ALL_CAPS in Go names; use CamelCase
keys/keypair.go:63:2: don't use ALL_CAPS in Go names; use CamelCase
keys/keypair.go:64:2: don't use ALL_CAPS in Go names; use CamelCase
session/session.go:23:2: don't use ALL_CAPS in Go names; use CamelCase
session/session.go:24:2: don't use ALL_CAPS in Go names; use CamelCase
session/session.go:25:2: don't use ALL_CAPS in Go names; use CamelCase
session/session.go:29:6: type name will be used as session.SessionCallbacks by other packages, and that stutters; consider calling this Callbacks
session/session.go:210:26: method GetRemoteId should be GetRemoteID
session/session_test.go:20:27: method GetPublicKeyForId should be GetPublicKeyForID

  • Add documentation comments

golint now requires all exported types, methods, and constants to have documentation comments on them. And the comments should have a particular style, starting with the identifier they describe. I've added some basic comments for all structs and methods.

  • Unexport implementation details

Secure Message mode constants should not be exported, they are for internal use. Spell them with a lowercase initial letter instead of uppercase.

  • Replace snake_case with camelCase

Go code style requires to use camelCase for variables, not snake_case.

  • Spell ID as "ID", not "Id"

golint warns us about incorrect spelling, fix it where possible.

  • Drop explicit zero initialization

Values are zero-initialized by default, golint warns about explicit zero initialization. Remove it in Secure Cell code.

golint now requires all exported types, methods, and constants to
have documentation comments on them. And the comments should have
a particular style, starting with the identifier they describe.
These constants should not be exported, they are for internal use.
Spell them with a lowercase initial letter instead of uppercase.
Go code style requires to use camelCase for variables, not snake_case.
golint warns us about incorrect spelling, fix it where possible.

Don't rename GetRemoteId and GetPublicKeyForId methods to avoid
breaking backward compatibility.
Values are zero-initialized by default, golint warns about explicit
zero initialization. Remove it.
@ilammy ilammy added the W-GoThemis 🐹 Wrapper: GoThemis, Go API label Mar 11, 2019
There are more Go files which may trigger golint spellchecker. Fix
identified casing there as well.
@vixentael vixentael removed the request for review from ignatk March 11, 2019 20:28
Copy link
Contributor

@vixentael vixentael left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good except backwards compatibility break for SecureMessage, plz remove it from current PR

@vixentael
Copy link
Contributor

Also: shall update all code in Wiki examples to use new syntax

@ilammy
Copy link
Collaborator Author

ilammy commented Mar 11, 2019

The constants in gothemis/message/message.go which are renamed were added only recently in #400. I did not intend them to be exported (and at that time I was not aware of Golang's scoping rules).

Since we did not expose this API in 0.10, I think we're okay with renaming the constants directly right now: the users will not see them in 0.11 so this should not break existing code.

@vixentael
Copy link
Contributor

oh, if they are not exposed to the end users – I'm quite happy then :)

Copy link
Collaborator

@Lagovas Lagovas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ilammy ilammy merged commit 9ab951e into cossacklabs:master Mar 12, 2019
@ilammy ilammy deleted the fix-golint branch March 12, 2019 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
W-GoThemis 🐹 Wrapper: GoThemis, Go API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants