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 github.com/mgechev/revive #7472

Merged
merged 7 commits into from
Aug 2, 2024

Conversation

purnesh42H
Copy link
Contributor

@purnesh42H purnesh42H commented Aug 1, 2024

cc #7444

RELEASE NOTES: None

@purnesh42H purnesh42H added the Type: Meta Github repo, process, etc label Aug 1, 2024
@purnesh42H purnesh42H added this to the 1.66 Release milestone Aug 1, 2024
Copy link

codecov bot commented Aug 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.51%. Comparing base (6fa393c) to head (b8af6c4).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7472      +/-   ##
==========================================
+ Coverage   81.38%   81.51%   +0.12%     
==========================================
  Files         354      357       +3     
  Lines       27080    27242     +162     
==========================================
+ Hits        22040    22207     +167     
+ Misses       3831     3826       -5     
  Partials     1209     1209              

see 21 files with indirect coverage changes

@purnesh42H purnesh42H changed the title Add github.com/mgechev/revive tools: Add github.com/mgechev/revive Aug 1, 2024
@purnesh42H purnesh42H force-pushed the add-revive-linter branch 6 times, most recently from 69ef453 to 1880907 Compare August 1, 2024 18:15
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

Tactically speaking...

Warnings:
  546  unused-parameter      
   83  exported              
   28  var-naming            
   15  redefines-builtin-id  
    8  package-comments      
    8  empty-block           
    6  var-declaration       
    3  indent-error-flow     
    2  increment-decrement   
    1  context-as-argument   
    1  superfluous-else      

...we're going to want to fix the issues that have few instances of them ASAP. But we will probably want to make this a blocking checker before fixing all the "unused-parameter" items, since there are so many of them.

scripts/vet.sh Outdated
Comment on lines 171 to 175
# - Collection of revive linter analysis checks
REV_OUT="$(mktemp)"
revive -formatter unix ./... >"${REV_OUT}" || true

(noret_grep -v "[unused-parameter]" "${REV_OUT}" | not grep -v "\.pb\.go:") || true
Copy link
Member

Choose a reason for hiding this comment

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

Now this doesn't show us the other errors.

Maybe we can just make revive skip that check? I assume that's possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After switching to plain formatter, it works

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Meta Github repo, process, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants