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

chore: use canonical golangci-lint github action #12134

Closed
wants to merge 39 commits into from
Closed
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
d952235
Update lint.yml
faddat Jun 3, 2022
400fef3
fix suspicious join by using .Dir instead of .Join
faddat Jun 3, 2022
960919a
Merge branch 'main' into suspicious-join
faddat Jun 3, 2022
9dffa36
Update CHANGELOG.md
faddat Jun 3, 2022
edc57bb
Update .github/workflows/lint.yml
faddat Jun 3, 2022
4118316
Update .github/workflows/lint.yml
faddat Jun 3, 2022
9db408c
Update CHANGELOG.md
faddat Jun 3, 2022
0b66c4d
Merge branch 'main' into patch-9
faddat Jun 3, 2022
241a808
Merge branch 'main' into patch-9
faddat Jun 3, 2022
3e991f1
Merge branch 'main' into patch-9
faddat Jun 3, 2022
f022bd3
Merge branch 'main' into patch-9
faddat Jun 3, 2022
75ef457
Merge branch 'main' into patch-9
faddat Jun 3, 2022
d801c2f
Merge branch 'main' into patch-9
faddat Jun 3, 2022
d2cabb0
Update CHANGELOG.md
faddat Jun 3, 2022
30cb49a
Merge branch 'main' into patch-9
faddat Jun 7, 2022
9847ac8
Merge branch 'main' into patch-9
tac0turtle Jun 8, 2022
b08f344
Merge branch 'main' into patch-9
faddat Jun 8, 2022
deffbbc
Merge branch 'main' into patch-9
tac0turtle Jun 8, 2022
ea5438d
Merge branch 'main' into patch-9
faddat Jun 8, 2022
26036ef
Merge branch 'main' into patch-9
faddat Jun 9, 2022
95ac2de
Merge remote-tracking branch 'notional/suspicious-join' into HEAD
faddat Jun 9, 2022
075af32
Merge branch 'main' into patch-9
faddat Jun 9, 2022
a6f1498
fix fliepath issues in simapp
faddat Jun 9, 2022
18835a4
Merge branch 'patch-9' of https://github.com/faddat/basecoin into pat…
faddat Jun 9, 2022
bd5667e
Merge branch 'main' into patch-9
faddat Jun 9, 2022
18becdf
unfix suspicious join in testnet.go
faddat Jun 9, 2022
d382053
Merge branch 'patch-9' of https://github.com/faddat/basecoin into pat…
faddat Jun 9, 2022
388eff7
Merge branch 'main' into patch-9
faddat Jun 9, 2022
6b97db8
Merge branch 'patch-9' of https://github.com/faddat/basecoin into pat…
faddat Jun 9, 2022
ca29bf1
revert changes from fix suspicious join
faddat Jun 9, 2022
a53a342
I have marked two suspicious joins as nolint. There is a solution an…
faddat Jun 9, 2022
8961afe
Update lint.yml
faddat Jun 9, 2022
a503ef7
fumpt x/slashing/module.go
faddat Jun 9, 2022
c16f56e
Merge branch 'patch-9' of https://github.com/faddat/basecoin into pat…
faddat Jun 9, 2022
3ae1cc6
Merge branch 'main' into patch-9
faddat Jun 10, 2022
9e1cbb4
Merge branch 'main' into patch-9
faddat Jun 10, 2022
d78d968
fumpt
faddat Jun 10, 2022
0461757
Delete .gitpod.yml
faddat Jun 13, 2022
24c2175
Merge branch 'main' into patch-9
faddat Jun 20, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 11 additions & 26 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -1,41 +1,26 @@
name: Lint
# Lint runs golangci-lint over the entire cosmos-sdk repository
# This workflow is run on every pull request and push to main
# The `golangci` will pass without running if no *.{go, mod, sum} files have been changed.
name: golangci-lint
on:
pull_request:
push:
tags:
- v*
branches:
- main
pull_request:
permissions:
contents: read
# Optional: allow read access to pull request. Use with `only-new-issues` option.
# pull-requests: read
jobs:
golangci:
name: golangci-lint
name: lint
runs-on: ubuntu-latest
steps:
- uses: actions/setup-go@v3
with:
go-version: 1.18
- uses: technote-space/[email protected]
id: git_diff
with:
PATTERNS: |
**/**.go
go.mod
go.sum
Comment on lines -18 to -24
Copy link
Member

Choose a reason for hiding this comment

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

we usually opt to have this to not run this position of ci when there are no changes to the go code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I recommend that we run it all the time since the compute to do that costs nothing and the previous setup resulted in not linting at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

But there's no sense in running it if there are zero code changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I can roll with it, I’ll make that change I’m just worried that I could introduce another bug that reproduces the prior state

Copy link
Member

Choose a reason for hiding this comment

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

please revert this change, after this we can look at merging

- name: Get data from Go build cache
# if: env.GIT_DIFF
if: ${{ false }}
uses: actions/cache@v3
with:
path: |
~/go/pkg/mod
~/.cache/golangci-lint
~/.cache/go-build
key: ${{ runner.os }}-go-build-${{ hashFiles('**/go.sum') }}
- uses: actions/checkout@v3
- name: golangci-lint
if: env.GIT_DIFF
uses: golangci/golangci-lint-action@v3
Copy link
Member

Choose a reason for hiding this comment

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

I think we have to migrate away from this action since it doesn't support workspaces or multiple go.mods in a single repo for now.

with:
# Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
version: latest
args: --out-format=tab
skip-go-installation: true
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements
faddat marked this conversation as resolved.
Show resolved Hide resolved

* [#12134](https://github.com/cosmos/cosmos-sdk/pull/12134) Use canonical golangci-lint github action, set to use the latest golangci-lint at all times
* [#12089](https://github.com/cosmos/cosmos-sdk/pull/12089) Mark the `TipDecorator` as beta, don't include it in simapp by default.
* [#12153](https://github.com/cosmos/cosmos-sdk/pull/12153) Add a new `NewSimulationManagerFromAppModules` constructor, to simplify simulation wiring.

Expand All @@ -56,6 +57,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

* (linting) [#12135](https://github.com/cosmos/cosmos-sdk/pull/12135) Fix variable naming issues per enabled linters. Run gofumpt to ensure easy reviews of ongoing linting work.
* (linting) [#12132](https://github.com/cosmos/cosmos-sdk/pull/12132) Change sdk.Int to math.Int, run `gofumpt -w -l .`, and `golangci-lint run ./... --fix`
* (cli) [#12147](https://github.com/cosmos/cosmos-sdk/pull/12137/) Fix suspicious join on filepath.Join by using filepath.Dir to specify the folder.
* (cli) [#12127](https://github.com/cosmos/cosmos-sdk/pull/12127) Fix the CLI not always taking into account `--fee-payer` and `--fee-granter` flags.
* (migrations) [#12028](https://github.com/cosmos/cosmos-sdk/pull/12028) Fix v0.45->v0.46 in-place store migrations.
* (baseapp) [#12089](https://github.com/cosmos/cosmos-sdk/pull/12089) Include antehandler and runMsgs events in SimulateTx.
Expand Down
1 change: 1 addition & 0 deletions client/keys/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ func printCreate(cmd *cobra.Command, k *keyring.Record, showMnemonic bool, mnemo

// print mnemonic unless requested not to.
if showMnemonic {
//nolint:staticcheck // this comlplained of the use of Sprintf, and I did not know how to rectify it.
if _, err := fmt.Fprintf(cmd.ErrOrStderr(), fmt.Sprintf("\n**Important** write this mnemonic phrase in a safe place.\nIt is the only way to recover your account if you ever forget your password.\n\n%s\n", mnemonic)); err != nil {
return fmt.Errorf("failed to print mnemonic: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion client/tx/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ func Sign(txf Factory, name string, txBuilder client.TxBuilder, overwriteSig boo
if overwriteSig {
sigs = []signing.SignatureV2{sig}
} else {
sigs = append(prevSignatures, sig)
sigs = append(prevSignatures, sig) //nolint:gocritic // this simly doesn't look like something that should change because of linting.
}
if err := txBuilder.SetSignatures(sigs...); err != nil {
return err
Expand Down
1 change: 1 addition & 0 deletions runtime/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
// App can be used to create a hybrid app.go setup where some configuration is
// done declaratively with an app config and the rest of it is done the old way.
// See simapp/app.go for an example of this setup.
//nolint:unused // we should keep beginblockers and endblockers here.
type App struct {
*baseapp.BaseApp
ModuleManager *module.Manager
Expand Down
2 changes: 1 addition & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ import (
_ "github.com/cosmos/cosmos-sdk/client/docs/statik"
)

const appName = "SimApp"
const appName = "SimApp" //nolint:deadcode,unused // we need this

var (
// DefaultNodeHome default home directories for the application daemon
Expand Down
2 changes: 1 addition & 1 deletion simapp/simd/cmd/testnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ func calculateIP(ip string, i int) (string, error) {
}

func writeFile(name string, dir string, contents []byte) error {
writePath := filepath.Join(dir)
writePath := filepath.Join(dir) //nolint:gocritic // Nolinted because cannot currently remember the solution.
file := filepath.Join(writePath, name)

err := tmos.EnsureDir(writePath, 0o755)
Expand Down
2 changes: 2 additions & 0 deletions store/v2alpha1/multi/test_util.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//nolint:unused // this file is triggering many unused liter errors and shouldn't be linted for that reason.
package multi

import (
Expand Down Expand Up @@ -41,6 +42,7 @@ func (dbSaveVersionFails) SaveVersion(uint64) error { return errors.New("dbSaveV
func (db dbRevertFails) Revert() error {
fail := false
if len(db.failOn) > 0 {
//nolint:staticcheck // staticcheck doesn't understand this properly, and don't want to modify out of caution.
fail, db.failOn = db.failOn[0], db.failOn[1:]
}
if fail {
Expand Down
2 changes: 1 addition & 1 deletion testutil/network/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func initGenFiles(cfg Config, genAccounts []authtypes.GenesisAccount, genBalance
}

func writeFile(name string, dir string, contents []byte) error {
writePath := filepath.Join(dir)
writePath := filepath.Join(dir) //nolint:gocritic // Nolinted because cannot currently remember the solution.
file := filepath.Join(writePath, name)

err := tmos.EnsureDir(writePath, 0o755)
Expand Down
1 change: 1 addition & 0 deletions testutil/testdata_pulsar/query.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//nolint:revive,gocritic // later we could resolve this by changing how we generate things, but was not sure how this file is generated and if that woud pose a breaking change.
package testdata_pulsar

import (
Expand Down
1 change: 1 addition & 0 deletions testutil/testdata_pulsar/query.pulsar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions x/auth/client/testutil/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -1299,6 +1299,7 @@ func (s *IntegrationTestSuite) TestGetAccountsCmd() {

func TestGetBroadcastCommandOfflineFlag(t *testing.T) {
clientCtx := client.Context{}.WithOffline(true)
//nolint:staticcheck // this is intentionally setting a new value to clientCtx to affect later commands.
clientCtx = clientCtx.WithTxConfig(simapp.MakeTestEncodingConfig().TxConfig)

cmd := authcli.GetBroadcastCommand()
Expand Down Expand Up @@ -1789,6 +1790,7 @@ func (s *IntegrationTestSuite) TestAuxToFeeWithTips() {
tc.feePayerArgs...,
)

//nolint:gocritic // rewriting this as a switch statement didn't make sense.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is totally what I mean about codecov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

eg:

  • comments aren't new code lacking test coverage
  • nolint statements aren't new code lacking test coverage

if tc.expectErrBroadCast {
require.Error(err)
} else if tc.errMsg != "" {
Expand Down
1 change: 1 addition & 0 deletions x/authz/client/testutil/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,7 @@ func (s *IntegrationTestSuite) TestNewExecGrantAuthorized() {

out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args)
var response sdk.TxResponse
//nolint:gocritic // rewriting this into a switch statement didn't make sense.
if tc.expectErrMsg != "" {
s.Require().NoError(clientCtx.Codec.UnmarshalJSON(out.Bytes(), &response), out.String())
s.Require().Contains(response.RawLog, tc.expectErrMsg)
Expand Down
3 changes: 1 addition & 2 deletions x/authz/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ package authz
import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
)

// AccountKeeper defines the expected account keeper (noalias)
type AccountKeeper interface {
GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI
GetAccount(ctx sdk.Context, addr sdk.AccAddress) types.AccountI
NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) types.AccountI
SetAccount(ctx sdk.Context, acc types.AccountI)
}
Expand Down
1 change: 1 addition & 0 deletions x/feegrant/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress,
}

newExp, err := feeAllowance.ExpiresAt()
//nolint:gocritic // gocritic recommends making this a switch statement and this felt like taking things too far.
if err != nil {
return err
} else if newExp != nil && newExp.Before(ctx.BlockTime()) {
Expand Down
1 change: 1 addition & 0 deletions x/feegrant/migrations/v046/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ var (
// - <0x01><exp_bytes><len(grantee_address_bytes)><grantee_address_bytes><len(granter_address_bytes)><granter_address_bytes>
func FeeAllowancePrefixQueue(exp *time.Time, key []byte) []byte {
// no need of appending len(exp_bytes) here, `FormatTimeBytes` gives const length everytime.
//nolint:gocritic // we want to use append the way it's used here.
allowanceByExpTimeKey := append(FeeAllowanceQueueKeyPrefix, sdk.FormatTimeBytes(*exp)...)
return append(allowanceByExpTimeKey, key...)
}
Expand Down
1 change: 1 addition & 0 deletions x/gov/migrations/v046/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ func convertToNewVotes(oldVotes v1beta1.Votes) (v1.Votes, error) {
// - if only Options is set, or both Option & Options are set, we read from Options,
// - if Options is not set, and Option is set, we read from Option,
// - if none are set, we throw error.
//nolint:gocritic // ignore lint error about if-else-if
if oldVote.Options != nil {
newWVOs = make([]*v1.WeightedVoteOption, len(oldVote.Options))
for j, oldWVO := range oldVote.Options {
Expand Down
5 changes: 2 additions & 3 deletions x/group/client/cli/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,8 @@ func parseMembers(membersFile string) ([]group.MemberRequest, error) {

func execFromString(execStr string) group.Exec {
exec := group.Exec_EXEC_UNSPECIFIED
switch execStr {
case ExecTry:
exec = group.Exec_EXEC_TRY
if exec == group.Exec_EXEC_TRY {
return group.Exec_EXEC_TRY
}
return exec
}
Expand Down
1 change: 1 addition & 0 deletions x/group/client/testutil/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -2557,6 +2557,7 @@ func (s *IntegrationTestSuite) createGroupThresholdPolicyWithBalance(adminAddres
}

// fundAllGroupPolicies sends tokens to all group policies of a given group ID.
//nolint:unused // used in tests
func (s *IntegrationTestSuite) fundAllGroupPolicies(groupID string, tokens sdk.Coin) {
val := s.network.Validators[0]
clientCtx := val.ClientCtx
Expand Down