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

chore: enhance golangci-lint with code complexity and other measures #484

Merged
merged 9 commits into from
Dec 1, 2022

Conversation

mowies
Copy link
Member

@mowies mowies commented Nov 30, 2022

This PR

  • adds some more linters to golangci-lint:
    • cyclop: Measures and limits cyclomatic complexity
    • gocognit: Measures and limits cognitive complexity
    • funlen: Limits function line and statement length
  • this change will enable us to get rid of sonarcloud since we have everything covered by golangci-lint
  • adjusts makefile test commands to return aggregated and fully syntactically correct coverage reports

Notes:

  • golangci-lint will fail until all errors are fixed in a follow up ticket
  • for now golangci-lint will not make the pipeline go red since we cannot fix all errors right now

Closes #462

Linter issues will be fixed in #485.

@mowies mowies changed the title chore: Enhance golangci-lint with code complexity and other measures chore: enhance golangci-lint with code complexity and other measures Nov 30, 2022
@codecov
Copy link

codecov bot commented Nov 30, 2022

Codecov Report

Merging #484 (fd2857a) into main (ba6eadd) will increase coverage by 1.26%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #484      +/-   ##
==========================================
+ Coverage   59.68%   60.95%   +1.26%     
==========================================
  Files          28       30       +2     
  Lines        2101     2177      +76     
==========================================
+ Hits         1254     1327      +73     
- Misses        743      745       +2     
- Partials      104      105       +1     
Impacted Files Coverage Δ
...or/controllers/keptnworkloadinstance/controller.go 77.31% <0.00%> (-1.55%) ⬇️
operator/controllers/common/evaluationhandler.go 78.07% <0.00%> (-0.57%) ⬇️
operator/api/v1alpha1/common/common.go 100.00% <0.00%> (ø)
operator/api/v1alpha1/common/phases.go 100.00% <0.00%> (ø)
Flag Coverage Δ
component-tests 50.88% <ø> (-0.20%) ⬇️
keptn-lifecycle-operator 58.53% <ø> (+1.88%) ⬆️
scheduler 4.52% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

remove redundant newlines

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

remove code quality tools reporting

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

adjust funlen go linter

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

also trigger golangci-lint on pushes to main and maintenance branches

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

set up complexity and function length measure for golangci-lint

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

comment in stuff

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

more debugging

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

more debugging

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

debugging

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

remove trailing slashes

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

remove prefixes on coverage reports

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

even more trys

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

one more

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

another try...

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

fix coverage files in makefile

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

one more try

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

remove lines that are not needed again

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

remove unneeded lines from coverage reports

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

comment out stuff

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

upload to code climate as well

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

adjust path

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

set up token correctly

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

rename

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

change coverage reporting

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

remove bug

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>

report coverage to codacy

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
@mowies mowies marked this pull request as ready for review November 30, 2022 13:44
Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
Copy link
Contributor

@philipp-hinteregger philipp-hinteregger left a comment

Choose a reason for hiding this comment

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

LGTM

.golangci.yml Outdated Show resolved Hide resolved
@odubajDT
Copy link
Contributor

looks good, but is there maybe a possibility to disable the checks on the *_test.go files ? Now we have errors that some test functions are too long, which is completely ok for us, as we are mostly using table tests

Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
Co-authored-by: odubajDT <93584209+odubajDT@users.noreply.github.com>
Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
odubajDT
odubajDT previously approved these changes Nov 30, 2022
Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
odubajDT
odubajDT previously approved these changes Dec 1, 2022
@mowies mowies marked this pull request as draft December 1, 2022 12:36
Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
@mowies mowies marked this pull request as ready for review December 1, 2022 12:57
thisthat
thisthat previously approved these changes Dec 1, 2022
odubajDT
odubajDT previously approved these changes Dec 1, 2022
Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
@mowies mowies dismissed stale reviews from odubajDT and thisthat via 26fce62 December 1, 2022 14:06
Signed-off-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
@sonarcloud
Copy link

sonarcloud bot commented Dec 1, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@mowies mowies merged commit 1d711d0 into keptn:main Dec 1, 2022
@mowies mowies deleted the enhance-golinter branch December 1, 2022 14:34
@keptn-bot keptn-bot mentioned this pull request Dec 1, 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.

Research: Investigate possibility of importing golangci-lint and go coverage reports into Sonarcloud
4 participants