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

meta: suggestion: require govet for linting #1370

Closed
ValarDragon opened this issue Jun 25, 2018 · 1 comment
Closed

meta: suggestion: require govet for linting #1370

ValarDragon opened this issue Jun 25, 2018 · 1 comment
Labels

Comments

@ValarDragon
Copy link
Contributor

Do we want to use go vet in the linter? Here are some sample things it wants to fix:

		accAuth.Coins = sdk.Coins{
			{genTx.Name + "Token", sdk.NewInt(1000)},
			{"steak", freeFermionsAcc},
		}

to

		accAuth.Coins = sdk.Coins{
			{Denom: genTx.Name + "Token", Amount: sdk.NewInt(1000)},
			{Denom: "steak", Amount: freeFermionsAcc},
		}

(The objection here was only because the struct definition was nested)

And it noticed that some json tags were broken, we had:
client/tx/sign.go:14::error: struct field tag json="name"``
it should have been json:"name".

I think it may be useful, but I don't feel strongly about it. (The main benefit being it catching the second bug) We currently spend alot of reviews making nitpicks on stuff that a linter in principle could catch,

Here's the golang page describing govet: https://golang.org/cmd/vet/

Optionally, we can turn off its composite literal check, since I think thats the least impactful of the things it provides. (I personally like it, but I don't want that being annoying to detract from what else it can help with)

@ValarDragon ValarDragon changed the title meta: Require govet for linting meta: suggestion: require govet for linting Jun 25, 2018
@ValarDragon
Copy link
Contributor Author

Closing in favor of 1417

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

No branches or pull requests

1 participant