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

fix(lint): Fix the lint issues discovered by golangci-lint #292

Closed
wants to merge 1 commit into from

Conversation

prabhu43
Copy link

Signed-off-by: Prabhu Jayakumar [email protected]

What this PR does / why we need it:
Fixes lint issues discovered by golangci-lint so that golangci-lint can be introduced in CI

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #291 , also allows to fix #276

Special notes for your reviewer:

Checklist:

  • Fixes #
  • Labelled this PR & related issue with documentation tag
  • PR messages has document related information
  • Labelled this PR & related issue with breaking-changes tag
  • PR messages has breaking changes related information
  • Labelled this PR & related issue with requires-upgrade tag
  • PR messages has upgrade related information
  • Commit has unit tests
  • Commit has integration tests

@codecov
Copy link

codecov bot commented Oct 22, 2020

Codecov Report

Merging #292 into master will decrease coverage by 0.23%.
The diff coverage is 62.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #292      +/-   ##
==========================================
- Coverage   62.04%   61.81%   -0.24%     
==========================================
  Files           6        6              
  Lines         469      474       +5     
==========================================
+ Hits          291      293       +2     
- Misses        136      138       +2     
- Partials       42       43       +1     
Impacted Files Coverage Δ
...g/controller/chaosengine/chaosengine_controller.go 54.94% <62.50%> (-0.23%) ⬇️

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 a16249f...3c3a75e. Read the comment docs.

@rahulchheda
Copy link
Member

@prabhu43 Sorry for the late response, but can you add golangci-lint as part of lint makefile target rather than creating a new one.
cc: @ksatchit

@prabhu43
Copy link
Author

@rahulchheda I could not see the target lint in the makefile. Can you help me to find it in the repo
https://github.com/litmuschaos/chaos-operator/blob/master/Makefile

@rahulchheda
Copy link
Member

@prabhu43 Seems like it is only in dev branch. ref: https://github.com/litmuschaos/chaos-operator/blob/dev/Makefile#L55.
Lets not sync it, but we should add more good lints.
Btw, golang-ci is now deprecated, no need to add that now, lets add this: https://github.com/mgechev/revive, and goimports w/ github.com/litmuschaos as local. Make sure you add these things in a file .golangci-lint or something else. Take reference from revive.
Ping me if you have any other doubts.

@ispeakc0de ispeakc0de closed this Aug 22, 2023
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.

Add golangci-lint integration in CI Fix lint issues discovered by golangci-lint
3 participants