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

Add golangci-lint to CI. #947

Merged
merged 7 commits into from Mar 30, 2022
Merged

Add golangci-lint to CI. #947

merged 7 commits into from Mar 30, 2022

Conversation

shiqizng
Copy link
Contributor

Summary

Add linting to project.

@codecov-commenter
Copy link

codecov-commenter commented Mar 29, 2022

Codecov Report

Merging #947 (99acbb9) into develop (99ab66f) will increase coverage by 0.32%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #947      +/-   ##
===========================================
+ Coverage    59.21%   59.53%   +0.32%     
===========================================
  Files           42       41       -1     
  Lines         5720     5689      -31     
===========================================
  Hits          3387     3387              
+ Misses        1896     1865      -31     
  Partials       437      437              
Impacted Files Coverage Δ
util/diff.go

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99ab66f...99acbb9. Read the comment docs.

@shiqizng shiqizng requested review from winder and cce March 29, 2022 18:32

issues:
# use these new lint checks on code since #2574
new-from-rev: 99ab66fdadcd77fef4048bb2aa86cae1f06c6e08
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll want to remove this if you have already fixed all the linting issues in this codebase — this references a commit on the go-algorand repo that I used to ignore all lint issues before this commit. (Because it was too much work to fix all the issues in the code before that date)

Copy link
Contributor

Choose a reason for hiding this comment

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

oh I see 99ab66f is a commit on indexer, well, never mind

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should update the description.

.golangci.yml Outdated

issues:
# use these new lint checks on code since #2574
new-from-rev: 988ad6384783701d0b78bed903ff1bc28277f05c
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it a lot of work to make indexer clean for the error-level linters you've picked out? just curious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if we start from this commit, there are just a couple of errors.

@winder
Copy link
Contributor

winder commented Mar 29, 2022

@cce should we pull in this change also? algorand/go-algorand#3734

Looks like there were some side effects on go-algorand that might not apply to Indexer.

@cce
Copy link
Contributor

cce commented Mar 30, 2022

sure, the gci linter is great!

@winder winder marked this pull request as ready for review March 30, 2022 17:45
Copy link
Contributor

@winder winder left a comment

Choose a reason for hiding this comment

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

LGTM, lets try it out

@winder winder merged commit ac25054 into develop Mar 30, 2022
@winder winder deleted the shiqi/linting branch March 30, 2022 17:47
@winder winder changed the title add linting Add golangci-lint to CI. Mar 30, 2022
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.

None yet

4 participants