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.

Sign up for GitHub

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

Jump to bottom

golangci: enable unused parameters linter #6018

Merged
merged 6 commits into from
Aug 11, 2023
Merged

golangci: enable unused parameters linter #6018

merged 6 commits into from
Aug 11, 2023

Conversation

pysel
Copy link
Member

@pysel pysel commented Aug 10, 2023

Closes: #6016

What is the purpose of the change

Enable unused parameters (unparam) linter in golangci and fix warnings

Testing and Verifying

NA

Documentation and Release Note

NA

@github-actions github-actions bot added C:CLI C:x/gamm Changes, features and bugs related to the gamm module. T:CI C:x/incentives C:x/superfluid C:app-wiring Changes to the app folder C:x/tokenfactory C:simulator Edits simulator or simulations C:x/concentrated-liquidity C:x/poolmanager labels Aug 10, 2023
@github-actions
Copy link
Contributor

Important Notice

This PR includes modifications to the tests/e2e/initialization module.
Please follow the instructions below:

  1. Backport these changes to the previous Osmosis version's branch.
  2. Run the script inside a Docker container to update genesis and configs for pre-upgrade Osmosis.
  3. Merge the backported changes.
  4. The image will be built and uploaded to Docker Hub here.
  5. Grab the latest image and update it in the PR to the main branch replacing the previousVersionInitTag in the osmosis/tests/e2e/containers/config.go

Please let us know if you need any help.

@pysel pysel added V:state/compatible/no_backport State machine compatible PR, depends on prior breaks A:no-changelog V:state/compatible/backport State machine compatible PR, should be backported A:backport/v16.x backport patches to v16.x branch and removed V:state/compatible/no_backport State machine compatible PR, depends on prior breaks labels Aug 10, 2023
@pysel pysel marked this pull request as ready for review August 10, 2023 19:10
pysel
pysel commented Aug 10, 2023
View reviewed changes
x/protorev/client/cli/query.go
@@ -158,6 +158,8 @@ func NewQueryPoolCmd() (*osmocli.QueryDescriptor, *types.QueryGetProtoRevPoolReq
}

// convert a string array "[1,2,3]" to []uint64
//
//nolint:unparam
Copy link
Member Author

Choose a reason for hiding this comment

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

note to reviewers: nolint due to design of osmocli

p0mvn
p0mvn approved these changes Aug 10, 2023
View reviewed changes
Copy link
Member

@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.

LGTM

pysel
pysel commented Aug 10, 2023
View reviewed changes
simulation/executor/util.go
@@ -8,6 +8,7 @@ import (
"github.com/osmosis-labs/osmosis/v17/simulation/simtypes"
)

//nolint:unparam
Copy link
Member Author

Choose a reason for hiding this comment

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

note to reviewer: nolint because t might be used in the future

pysel
pysel commented Aug 10, 2023
View reviewed changes
x/downtime-detector/client/cli/query.go
@@ -30,6 +30,7 @@ downtime-duration is a duration, but is restricted to a smaller set. Heres a few
}, &queryproto.RecoveredSinceDowntimeOfLengthRequest{}
}

//nolint:unparam
Copy link
Member Author

Choose a reason for hiding this comment

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

note to reviewers: nolint due to design of osmocli

@p0mvn p0mvn mentioned this pull request Aug 10, 2023
Merged
6 tasks
@pysel
Copy link
Member Author

pysel commented Aug 10, 2023

devbot changelog add api golangci: add unused parameters linter

@pysel
Copy link
Member Author

pysel commented Aug 10, 2023

devbot help

@pysel pysel added V:state/breaking State machine breaking PR V:state/compatible/backport State machine compatible PR, should be backported A:backport/v16.x backport patches to v16.x branch and removed A:backport/v16.x backport patches to v16.x branch V:state/compatible/backport State machine compatible PR, should be backported V:state/breaking State machine breaking PR labels Aug 10, 2023
@devbot-wizard
Copy link
Collaborator

Hi! I'm DevBot, a bot that helps with common tasks in the development process. Commands:

  • devbot fix: Fix basic errors in the PR. (e.g. imports, changelog conflicts, go mod conflicts)
  • devbot merge base: Merge the base branch into the PR branch.
  • devbot add changelog [misc/feat/bug/api/sec] [message]: Add a changelog entry to the PR. (e.g. devbot add changelog feat Added a new feature)
    • If message is blank, defaults to PR title.
  • devbot re-pr: Re-PR the PR, useful for external contributors where we have no edit perms.

czarcas7ic
czarcas7ic approved these changes Aug 11, 2023
View reviewed changes
Copy link
Member

@czarcas7ic czarcas7ic left a comment

Choose a reason for hiding this comment

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

Nice!

@czarcas7ic czarcas7ic merged commit 2e3f19d into main Aug 11, 2023
@czarcas7ic czarcas7ic deleted the pycel/unused-arg-linter branch August 11, 2023 22:14
mergify bot pushed a commit that referenced this pull request Aug 11, 2023
@pysel @mergify
golangci: enable unused parameters linter (#6018)
cdc5d5b 
* enable unused parameters linter

* enable unparam linter and fix basic unparam warnings

* finish with unparam

* reset deliverTx and mark as nolint

* nolint no favor

* changelog

(cherry picked from commit 2e3f19d)

# Conflicts:
#	x/concentrated-liquidity/client/cli/tx.go
#	x/protorev/keeper/rebalance.go
#	x/superfluid/keeper/migrate.go
@mergify mergify bot mentioned this pull request Aug 11, 2023
Closed
p0mvn pushed a commit that referenced this pull request Aug 29, 2023
@pysel @p0mvn
golangci: enable unused parameters linter (#6018)
57fdc6a 
* enable unused parameters linter

* enable unparam linter and fix basic unparam warnings

* finish with unparam

* reset deliverTx and mark as nolint

* nolint no favor

* changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@czarcas7ic czarcas7ic czarcas7ic approved these changes

@p0mvn p0mvn p0mvn approved these changes

@mattverse mattverse Awaiting requested review from mattverse mattverse is a code owner

@nicolaslara nicolaslara Awaiting requested review from nicolaslara nicolaslara is a code owner

@stackman27 stackman27 Awaiting requested review from stackman27

@AlpinYukseloglu AlpinYukseloglu Awaiting requested review from AlpinYukseloglu AlpinYukseloglu is a code owner

Assignees
No one assigned
Labels
A:backport/v16.x backport patches to v16.x branch C:app-wiring Changes to the app folder C:CLI C:simulator Edits simulator or simulations C:x/concentrated-liquidity C:x/gamm Changes, features and bugs related to the gamm module. C:x/incentives C:x/poolmanager C:x/superfluid C:x/tokenfactory T:CI V:state/compatible/backport State machine compatible PR, should be backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

lint: unused args
4 participants
@pysel @devbot-wizard @p0mvn @czarcas7ic