chore: Address linting issues #12851

faddat · 2022-08-08T13:53:06Z

Description

This PR addresses linting issues prior to go 1.19 usage.

change to gofumpt to avoid conflicts between nolintlint and gofmt and goimports
run linter with go 1.19
lint the repository

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

alexanderbez · 2022-08-08T21:46:48Z

Awesome work and thank you!!!

tac0turtle · 2022-08-09T08:03:56Z

CHANGELOG.md

@@ -45,6 +45,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

 ### Improvements

+* (ci) [#12851](https://github.com/cosmos/cosmos-sdk/pull/12851) lint grandfathered issues


don't think this is needed

api/cosmos/base/query/v1beta1/pagination.pulsar.go

tac0turtle · 2022-08-09T08:07:46Z

cosmovisor/go.mod

@@ -3,7 +3,7 @@ module github.com/cosmos/cosmos-sdk/cosmovisor
 go 1.18

 require (
-	github.com/cosmos/cosmos-sdk v0.46.0
+	github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20220808215643-b6791b7af44b


should be 0.46.0

ah, I see. This can cause a thing....

Will take me a moment to figure out.

Yeah this is because you did go work sync after having added cosmovisor in the go.work

@faddat, meaculpa, removing this was breaking make test. We have a few solutions to fix that:

In run-tests (Makefile), add GOWORK=off before running go test. This requires (although I think it's good), packages to be properly tagged to pass CI.

Accept that it will upgrade the tendermint version until the SDK is downgrading to 34.x on main as well (as the support of 0.35 is anyway dropped by TM)

I am actually for doing both, merging with cosmovisor in the go.work but then still disable it in the CI runs to force us to tag properly our versions. Wdyt @faddat, @marbar3778?

tac0turtle · 2022-08-09T08:18:57Z

go.work

@@ -5,6 +5,7 @@ use (
 	./api
 	./client/v2
 	./core
+	./cosmovisor


I think we decided to remove this? @julienrbrt you did something related to this no?

It will likely need to be re-removed, then its removal will cause some kind of issue I don't fully understand.

Your issue was in go 1.19 PR you said right? So it won't happen in this PR anyway :)

#12851 (comment)

tac0turtle · 2022-08-09T08:25:39Z

x/gov/client/testutil/grpc.go

-	dp := v1.NewDepositParams(params.MinDeposit, params.MaxDepositPeriod)
-	vp := v1.NewVotingParams(params.VotingPeriod)
-	tp := v1.NewTallyParams(params.Quorum, params.Threshold, params.VetoThreshold)
+	dp := v1.NewDepositParams(params.MinDeposit, params.MaxDepositPeriod)          //nolint:staticcheck // TODO: refactor to only refer to Params


why the todos

wasn't sure how to fix-- also if to fix.

Is fix simply getting rid of everything that works like params.Thingy ?

(or, is it just wrong, and the todos aren't needed?)

faddat · 2022-08-09T09:02:39Z

if this passes 00 and 01 I'd say that it is ready.

also, the diff between the most recent commit and the one before might be interesting, as I don't think that anything that was changed, should have affected those tests.

faddat · 2022-08-09T09:05:55Z

That did not work.

let's see where this one leaves us-- it reverts changes to snapshots.

I am flummoxed.

faddat · 2022-08-09T09:11:35Z

Apologies. I think that there is something deficient in this branch, and that it is most prudent to do-over from main.

julienrbrt · 2022-08-09T23:45:19Z

Superseded by #12867

use the linter

c37ed87

faddat requested a review from a team as a code owner August 8, 2022 13:53

github-actions bot added the Type: CI label Aug 8, 2022

Update lint.yml

d540c22

faddat changed the title ~~chore: use the linter~~ chore: Use canonical golangci-lint github action Aug 8, 2022

gofumpt

16289ed

github-actions bot added C:Cosmovisor Issues and PR related to Cosmovisor C:x/auth C:x/bank C:x/evidence C:x/gov C:x/upgrade labels Aug 8, 2022

golangci-lint run ./... --fix

45396dc

github-actions bot added C:CLI C:Keys Keybase, KMS and HSMs C:Simulations C:Store C:x/authz C:x/crisis C:x/distribution distribution module related C:x/feegrant C:x/genutil genutil module issues C:x/group C:x/slashing C:x/staking labels Aug 8, 2022

faddat added 2 commits August 8, 2022 21:10

repair handler.go

a3817e1

restore staking

fb489fa

github-actions bot removed the C:x/staking label Aug 8, 2022

linting complete

73ca893

github-actions bot added the C:orm label Aug 8, 2022

faddat marked this pull request as draft August 8, 2022 17:32

faddat changed the title ~~chore: Use canonical golangci-lint github action~~ chore: Address linting issues Aug 9, 2022

Merge remote-tracking branch 'origin/main' into use-linter

d0a8f88

faddat force-pushed the use-linter branch from 7478638 to d0a8f88 Compare August 9, 2022 04:35

faddat added 2 commits August 9, 2022 11:46

sync

33d317a

attempt fix cosmovisor

9b7fac1

faddat marked this pull request as ready for review August 9, 2022 05:29

faddat added 2 commits August 9, 2022 12:48

Update CHANGELOG.md

4de66c4

Merge branch 'main' into use-linter

ba566a6

tac0turtle reviewed Aug 9, 2022

View reviewed changes

api/cosmos/base/query/v1beta1/pagination.pulsar.go Outdated Show resolved Hide resolved

Update CHANGELOG.md

6639398

tac0turtle reviewed Aug 9, 2022

View reviewed changes

faddat added 3 commits August 9, 2022 15:09

check api/cosmos out of main

9f05008

possible store v2 issue?

e5be8ba

cosmovisor fix?

99c01b8

tac0turtle approved these changes Aug 9, 2022

View reviewed changes

revert all changes to store.

c856952

revert snapshots folder

c072169

faddat marked this pull request as draft August 9, 2022 09:11

faddat mentioned this pull request Aug 9, 2022

chore: no-ioutil and make proto-gen #12866

Closed

19 tasks

julienrbrt closed this Aug 9, 2022

This was referenced Aug 10, 2022

fix: re-add cosmovisor in go.work to fix make test #12884

Merged

docs(cosmovisor): updates docs about path name #12921

Merged

julienrbrt mentioned this pull request Aug 17, 2022

build: ensure GOWORK=off make test passes #12951

Merged

19 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Address linting issues #12851

chore: Address linting issues #12851

faddat commented Aug 8, 2022 •

edited

Loading

alexanderbez commented Aug 8, 2022

tac0turtle Aug 9, 2022

faddat Aug 9, 2022

tac0turtle Aug 9, 2022

faddat Aug 9, 2022

julienrbrt Aug 9, 2022

julienrbrt Aug 9, 2022 •

edited

Loading

julienrbrt Aug 9, 2022

tac0turtle Aug 9, 2022

faddat Aug 9, 2022

julienrbrt Aug 9, 2022 •

edited

Loading

julienrbrt Aug 9, 2022

tac0turtle Aug 9, 2022

faddat Aug 9, 2022

faddat Aug 9, 2022

faddat commented Aug 9, 2022

faddat commented Aug 9, 2022

faddat commented Aug 9, 2022

julienrbrt commented Aug 9, 2022

		@@ -45,6 +45,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

		### Improvements

		* (ci) [#12851](https://github.com/cosmos/cosmos-sdk/pull/12851) lint grandfathered issues

chore: Address linting issues #12851

chore: Address linting issues #12851

Conversation

faddat commented Aug 8, 2022 • edited Loading

Description

Author Checklist

Reviewers Checklist

alexanderbez commented Aug 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julienrbrt Aug 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julienrbrt Aug 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

faddat commented Aug 9, 2022

faddat commented Aug 9, 2022

faddat commented Aug 9, 2022

julienrbrt commented Aug 9, 2022

faddat commented Aug 8, 2022 •

edited

Loading

julienrbrt Aug 9, 2022 •

edited

Loading

julienrbrt Aug 9, 2022 •

edited

Loading