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

tools: Add unparam linter #1443

Merged
merged 10 commits into from
Jun 29, 2018
Merged

tools: Add unparam linter #1443

merged 10 commits into from
Jun 29, 2018

Conversation

ValarDragon
Copy link
Contributor

unparam detects unused parameters in functions, and a parameter to
a function which only ever takes on one value. The latter is an
indication that more tests are required.

There are many nolints in this PR, as I believe that writing tests
to fix alot of these situations is out of scope for this PR / it
will be changed in future commits. There are some nolints for
when we have to comply to normal api's.

ref #1417 , closes #1084

  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG.md
  • Updated Gaia/Examples
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)
  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)

unparam detects unused parameters in functions, and a parameter to
a function which only ever takes on one value. The latter is an
indication that more tests are required.

There are many nolints in this PR, as I believe that writing tests
to fix alot of these situations is out of scope for this PR / it
will be changed in future commits. There are some nolints for
when we have to comply to normal api's.
@codecov
Copy link

codecov bot commented Jun 29, 2018

Codecov Report

❗ No coverage uploaded for pull request base (develop@47e4682). Click here to learn what that means.
The diff coverage is 64%.

@@            Coverage Diff             @@
##             develop    #1443   +/-   ##
==========================================
  Coverage           ?   63.09%           
==========================================
  Files              ?      118           
  Lines              ?     6563           
  Branches           ?        0           
==========================================
  Hits               ?     4141           
  Misses             ?     2151           
  Partials           ?      271

@rigelrozanski
Copy link
Contributor

Amazing - it's shocking how many parameters where left behind - I think this linter is going to save a lot of energy for us.

@rigelrozanski
Copy link
Contributor

p.s. this is hilariously annoying updating the branch and then waiting for it to pass, and then having to update the branch again - wish there was a "run tests and if they pass, merge immediately" button

@ValarDragon
Copy link
Contributor Author

Not sure why upload coverage hasn't terminated :/

@rigelrozanski rigelrozanski merged commit 097dd8a into develop Jun 29, 2018
@rigelrozanski rigelrozanski deleted the dev/lint_unparam branch June 29, 2018 22:22
adrianbrink pushed a commit that referenced this pull request Jul 2, 2018
* tools: Add unparam linter

unparam detects unused parameters in functions, and a parameter to
a function which only ever takes on one value. The latter is an
indication that more tests are required.

There are many nolints in this PR, as I believe that writing tests
to fix alot of these situations is out of scope for this PR / it
will be changed in future commits. There are some nolints for
when we have to comply to normal api's.

* crypto/keys no longer used by x/gov/client/rest/rest.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check for unused function inputs
3 participants