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

imp: enable revive rules #400

Closed
wants to merge 41 commits into from
Closed

imp: enable revive rules #400

wants to merge 41 commits into from

Conversation

fedekunze
Copy link
Contributor

@fedekunze fedekunze commented Mar 16, 2022

Description

I have set all severity: error flags to a warning.
This in the meantime will enable us to tackle each of the lint issues in individual PRs.

Closes: #XXX


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.

PR review checkboxes:

I have...

  • added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • included the correct type prefix in the PR title
  • targeted the correct branch (see PR Targeting)
  • provided a link in the PR description to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all required CI checks have passed

Code maintenance:

I have...

  • written unit and integration tests
  • added relevant godoc and code comments.
  • updated relevant documentation (docs/) or specification (x/<module>/spec/)

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 all author checklist items have been addressed
  • confirmed that this PR does not change production code

@github-actions github-actions bot added the CI label Mar 16, 2022
@codecov
Copy link

codecov bot commented Mar 19, 2022

Codecov Report

Merging #400 (cb108ea) into main (7649ee0) will increase coverage by 0.07%.
The diff coverage is 98.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #400      +/-   ##
==========================================
+ Coverage   82.57%   82.64%   +0.07%     
==========================================
  Files         122      122              
  Lines        6885     6914      +29     
==========================================
+ Hits         5685     5714      +29     
  Misses       1055     1055              
  Partials      145      145              
Impacted Files Coverage Δ
x/erc20/types/proposal.go 91.46% <50.00%> (ø)
x/claims/keeper/ibc_callbacks.go 97.76% <100.00%> (ø)
x/epochs/keeper/abci.go 100.00% <100.00%> (ø)
x/epochs/types/identifier.go 100.00% <100.00%> (ø)
x/erc20/keeper/evm.go 92.15% <100.00%> (ø)
x/erc20/types/token_pair.go 100.00% <100.00%> (ø)
x/incentives/keeper/gas_meters.go 97.10% <100.00%> (ø)
x/inflation/types/params.go 88.57% <100.00%> (-0.32%) ⬇️
x/vesting/keeper/msg_server.go 91.50% <100.00%> (+1.29%) ⬆️
x/vesting/types/clawback_vesting_account.go 96.77% <100.00%> (+0.02%) ⬆️
... and 1 more

@github-actions github-actions bot added the types label Mar 19, 2022
@fedekunze fedekunze added good first issue Good for newcomers help wanted Extra attention is needed labels Mar 22, 2022
Copy link
Contributor

@Prosp3r Prosp3r left a comment

Choose a reason for hiding this comment

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

Changes look good.
Just some questions for understanding.

@fedekunze
Copy link
Contributor Author

Thanks @Prosp3r for the review. Can you take over this PR to fix the linter that is currently failing because of all the new rules?

@Prosp3r
Copy link
Contributor

Prosp3r commented May 18, 2022

I have set all severity: error flags to a warning.
This in the meantime will enable us to tackle each of the lint issues in individual PRs.

@lgtm-com
Copy link

lgtm-com bot commented May 18, 2022

This pull request introduces 13 alerts when merging 25e5fcc into ffff90c - view on LGTM.com

new alerts:

  • 9 for Useless assignment to local variable
  • 4 for Unreachable statement

@lgtm-com
Copy link

lgtm-com bot commented May 19, 2022

This pull request introduces 13 alerts when merging 745d14a into d29dd33 - view on LGTM.com

new alerts:

  • 9 for Useless assignment to local variable
  • 4 for Unreachable statement

Copy link
Contributor

@danburck danburck left a comment

Choose a reason for hiding this comment

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

LGTM, @Prosp3r can you add a changelog and resolve the open conversations?

x/epochs/keeper/abci.go Show resolved Hide resolved
x/erc20/types/proposal.go Show resolved Hide resolved
@fedekunze
Copy link
Contributor Author

linter needs to be fixed too

@lgtm-com
Copy link

lgtm-com bot commented May 30, 2022

This pull request fixes 1 alert when merging 0b0abbb into 150082f - view on LGTM.com

fixed alerts:

  • 1 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented May 30, 2022

This pull request fixes 1 alert when merging 97cdf72 into 150082f - view on LGTM.com

fixed alerts:

  • 1 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented May 30, 2022

This pull request fixes 1 alert when merging a8d35b3 into 150082f - view on LGTM.com

fixed alerts:

  • 1 for Useless assignment to local variable

@Prosp3r Prosp3r enabled auto-merge (squash) May 30, 2022 10:53
@lgtm-com
Copy link

lgtm-com bot commented May 30, 2022

This pull request fixes 1 alert when merging 833e05f into 150082f - view on LGTM.com

fixed alerts:

  • 1 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented May 30, 2022

This pull request fixes 1 alert when merging 563a5cb into 150082f - view on LGTM.com

fixed alerts:

  • 1 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented May 30, 2022

This pull request fixes 1 alert when merging d595bb6 into 150082f - view on LGTM.com

fixed alerts:

  • 1 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented May 30, 2022

This pull request fixes 1 alert when merging 71c5161 into 8d53902 - view on LGTM.com

fixed alerts:

  • 1 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented May 30, 2022

This pull request fixes 1 alert when merging 485d71a into 8d53902 - view on LGTM.com

fixed alerts:

  • 1 for Useless assignment to local variable

@fedekunze
Copy link
Contributor Author

fedekunze commented May 31, 2022

@Prosp3r have you looked into the severity configuration and GitHub Actions output format?

This was referenced Jun 1, 2022
@lgtm-com
Copy link

lgtm-com bot commented Jun 2, 2022

This pull request fixes 1 alert when merging cb108ea into 7649ee0 - view on LGTM.com

fixed alerts:

  • 1 for Useless assignment to local variable

@danburck
Copy link
Contributor

Closing for now. Please reopen if it's being worked on

@danburck danburck closed this Jun 10, 2022
auto-merge was automatically disabled June 10, 2022 09:59

Pull request was closed

@fedekunze fedekunze deleted the fedekunze/revive-lint-rules branch February 6, 2023 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI good first issue Good for newcomers help wanted Extra attention is needed types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants