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

lint: enable unused in golangci-lint #1939

Merged
merged 1 commit into from
Jul 4, 2022
Merged

lint: enable unused in golangci-lint #1939

merged 1 commit into from
Jul 4, 2022

Conversation

p0mvn
Copy link
Member

@p0mvn p0mvn commented Jul 1, 2022

Follow-up to: #1897

What is the purpose of the change

We would like to improve the quality of our codebase by trying out components of the staticcheck linter.

This change introduced unused - 1st of the 4 components

Although, we added deadcode linter in #1934, unused seems to be detecting more unused logic.

In addition to removing the unused code, the following was done:

  • disable unused and deadcode for x/gamm/pool-models/stableswap since it is currently in development

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? no
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? no
  • How is the feature or change documented? not applicable

@github-actions github-actions bot added C:app-wiring Changes to the app folder C:x/gamm Changes, features and bugs related to the gamm module. C:x/incentives C:x/lockup C:x/superfluid C:x/tokenfactory T:CI labels Jul 1, 2022
Copy link
Member Author

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

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

TODOs to keep track of once this PR is accepted

@@ -95,5 +95,12 @@ issues:
- path: client/docs
linters:
- all
- path: x/gamm/pool-models/stableswap
linters:
# Stableswap is in development.
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: add issue reminding to re-enable this

x/gamm/keeper/pool_service.go Outdated Show resolved Hide resolved
sdk.NewAttribute(sdk.AttributeKeySender, msg.Sender),
),
})
server.keeper.createAddLiquidityEvent(ctx, sender, msg.PoolId, addedLiquidity)
Copy link
Member

Choose a reason for hiding this comment

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

Lets please separate out lint PRs from functionality changes / more detailed refactors like this

@@ -61,6 +61,7 @@ func NewKeeper(cdc codec.BinaryCodec, storeKey sdk.StoreKey, paramSpace paramtyp
}
}

// nolint: unused
func (k *Keeper) createSwapEvent(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) {
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: re-enable missing events

@p0mvn p0mvn marked this pull request as ready for review July 1, 2022 17:22
@p0mvn p0mvn requested a review from a team July 1, 2022 17:22
@faddat
Copy link
Member

faddat commented Jul 2, 2022

Strong support of this :)

Copy link
Member

@mattverse mattverse left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines -91 to -92
accountKeeper stakingtypes.AccountKeeper
bankKeeper stakingtypes.BankKeeper
Copy link
Member

Choose a reason for hiding this comment

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

Ah good catch that this was caught

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:app-wiring Changes to the app folder C:x/gamm Changes, features and bugs related to the gamm module. C:x/incentives C:x/lockup C:x/tokenfactory T:CI
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants