Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fully enable revive #3888

Merged
merged 106 commits into from
Aug 1, 2023
Merged

fully enable revive #3888

merged 106 commits into from
Aug 1, 2023

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Jun 18, 2023

This PR fully enables the revive linter.

So far it has been removing import shadwing
caused by the use of the word suite as a
method reciever instead of the letter s.

Summary of changes

  • suite -> s
  • standardized naming of variables
  • remove unused recievers
  • keep unused parameters
linters-settings:
  gci:
    sections:
      - standard # Standard section: captures all standard packages.
      - default # Default section: contains all imports that could not be matched to another section type.
      - blank # blank imports
      - dot # dot imports
      - prefix(cosmossdk.io)
      - prefix(github.com/cosmos/cosmos-sdk)
      - prefix(github.com/cometbft/cometbft)
      - prefix(github.com/cosmos/ibc-go)
    custom-order: true
  dogsled:
    max-blank-identifiers: 3
  maligned:
    # print struct with more effective memory layout or not, false by default
    suggest-new: true
  nolintlint:
    allow-unused: false
    allow-leading-space: true
    require-explanation: false
    require-specific: false
  revive:
    enable-all-rules: true
    rules:
      - name: if-return
        disabled: true
      - name: max-public-structs
        disabled: true
      - name: cognitive-complexity
        disabled: true
      - name: argument-limit
        disabled: true
      - name: cyclomatic
        disabled: true
      - name: file-header
        disabled: true
      - name: function-length
        disabled: true
      - name: function-result-limit
        disabled: true
      - name: line-length-limit
        disabled: true
      - name: flag-parameter
        disabled: true
      - name: add-constant
        disabled: true
      - name: empty-lines
        disabled: true
      - name: banned-characters
        disabled: true
      - name: deep-exit
        disabled: true
      - name: unused-parameter
        disabled: true
      - name: modifies-value-receiver
        disabled: true
      - name: early-return
        disabled: true
      - name: confusing-naming
        disabled: true
      - name: defer
        disabled: true
      - name: unhandled-error
        disabled: false
        arguments:
          - 'fmt.Printf'
          - 'fmt.Print'
          - 'fmt.Println'
          - 'myFunction'

@faddat
Copy link
Contributor Author

faddat commented Jun 18, 2023

@codecov-commenter
Copy link

codecov-commenter commented Jun 18, 2023

Codecov Report

Merging #3888 (f7adca4) into main (2ac5506) will decrease coverage by 0.03%.
The diff coverage is 65.68%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3888      +/-   ##
==========================================
- Coverage   79.49%   79.47%   -0.03%     
==========================================
  Files         188      188              
  Lines       12987    12997      +10     
==========================================
+ Hits        10324    10329       +5     
- Misses       2234     2239       +5     
  Partials      429      429              
Files Changed Coverage Δ
...interchain-accounts/controller/keeper/handshake.go 89.85% <ø> (ø)
...les/apps/27-interchain-accounts/host/ibc_module.go 91.11% <ø> (ø)
...ps/27-interchain-accounts/host/keeper/handshake.go 89.39% <ø> (ø)
...odules/core/02-client/migrations/v7/solomachine.go 18.30% <0.00%> (ø)
modules/core/02-client/types/proposal.go 87.01% <0.00%> (ø)
modules/core/23-commitment/types/merkle.go 68.04% <0.00%> (ø)
modules/core/keeper/msg_server.go 55.95% <0.00%> (ø)
modules/core/24-host/parse.go 51.47% <30.00%> (-2.63%) ⬇️
...dules/light-clients/06-solomachine/client_state.go 88.28% <33.33%> (ø)
modules/apps/29-fee/types/msgs.go 89.38% <50.00%> (ø)
... and 29 more

@faddat
Copy link
Contributor Author

faddat commented Jun 19, 2023

@crodriguezvega -- if you are reviewing, can you pull this branch, and then run:

golangci-lint run ./... --fix

?

I think this may have revealed an issue (the thing about return not doing anything inside defer)

@DimitrisJim DimitrisJim marked this pull request as ready for review July 31, 2023 07:20
Copy link
Contributor

@DimitrisJim DimitrisJim left a comment

Choose a reason for hiding this comment

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

Made a couple of changes here, mostly to keep the diff localized and minimal.

  1. Disable use-any. I think this is fine tbh but I don't think there's much benefit in enforcing a single usage. We can revisit at a later point if required.
  2. Renamed s to suite in the test files.
  3. Reverted the renaming of vars I.e _ -> someName and someName -> _. This usage was (and still is) inconsistent. A different issue should be opened to tackle this consistency nit.

Overall, LGTM, big thanks @faddat for the work on this and sorry for the slight delay in getting through with it (big diffs spook me).

@chatton
Copy link
Contributor

chatton commented Aug 1, 2023

Thank you @faddat and @DimitrisJim ❤️

@DimitrisJim DimitrisJim merged commit a4ca39c into cosmos:main Aug 1, 2023
@faddat
Copy link
Contributor Author

faddat commented Aug 3, 2023

oh amazing guys! I've been busy with family and thrilled to see this in <3

Copy link

@Apdlrcjafg19 Apdlrcjafg19 left a comment

Choose a reason for hiding this comment

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

linters-settings:
gci:
sections:
- standard # Standard section: captures all standard packages.
- default # Default section: contains all imports that could not be matched to another section type.
- blank # blank imports
- dot # dot imports
- prefix(cosmossdk.io)
- prefix(github.com/cosmos/cosmos-sdk)
- prefix(github.com/cometbft/cometbft)
- prefix(github.com/cosmos/ibc-go)
custom-order: true
dogsled:
max-blank-identifiers: 3
maligned:
# print struct with more effective memory layout or not, false by default
suggest-new: true
nolintlint:
allow-unused: false
allow-leading-space: true
require-explanation: false
require-specific: false
revive:
enable-all-rules: true
rules:
- name: if-return
disabled: true
- name: max-public-structs
disabled: true
- name: cognitive-complexity
disabled: true
- name: argument-limit
disabled: true
- name: cyclomatic
disabled: true
- name: file-header
disabled: true
- name: function-length
disabled: true
- name: function-result-limit
disabled: true
- name: line-length-limit
disabled: true
- name: flag-parameter
disabled: true
- name: add-constant
disabled: true
- name: empty-lines
disabled: true
- name: banned-characters
disabled: true
- name: deep-exit
disabled: true
- name: unused-parameter
disabled: true
- name: modifies-value-receiver
disabled: true
- name: early-return
disabled: true
- name: confusing-naming
disabled: true
- name: defer
disabled: true
- name: unhandled-error
disabled: false
arguments:
- 'fmt.Printf'
- 'fmt.Print'
- 'fmt.Println'
- 'myFunction'

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants