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

Enable gocritic and gocyclo linters #2236

Merged
merged 8 commits into from
Dec 4, 2023
Merged

Enable gocritic and gocyclo linters #2236

merged 8 commits into from
Dec 4, 2023

Conversation

ankitjain235
Copy link
Contributor

@ankitjain235 ankitjain235 commented Aug 3, 2023

Change Overview

  • Enable gocritic and gocyclo linters.
  • Fix issues detected

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • πŸ› Bugfix
  • 🌻 Feature
  • πŸ—ΊοΈ Documentation
  • πŸ€– Test

Issues

Test Plan

  • πŸ’ͺ Manual
  • ⚑ Unit test
  • πŸ’š E2E

@github-actions
Copy link

github-actions bot commented Aug 3, 2023

Thanks for submitting this pull request πŸŽ‰. The team will review it soon and get back to you.

If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document.

@infraq infraq added this to In Progress in Kanister Aug 3, 2023
@ankitjain235 ankitjain235 changed the title Enable gocyclo linter Enable gocyclo,gocritic linter Aug 3, 2023
@ankitjain235 ankitjain235 changed the title Enable gocyclo,gocritic linter Enable go linter - PR#1 Aug 3, 2023
@ankitjain235 ankitjain235 marked this pull request as ready for review August 3, 2023 15:30
@viveksinghggits
Copy link
Contributor

Let's mention the linter names in the PR title as well.

Kanister automation moved this from In Progress to Reviewer approved Aug 8, 2023
@pavannd1 pavannd1 changed the title Enable go linter - PR#1 Enable gocritic and gocyclo linters Aug 8, 2023
@@ -503,7 +501,7 @@ func parseGenericObjectReference(s string) (crv1alpha1.ObjectReference, error) {
}, nil
}

func parseObjectsFromSelector(selector, kind, sns string, cli kubernetes.Interface, osCli osversioned.Interface, parsed map[string]bool) ([]crv1alpha1.ObjectReference, error) {
func parseObjectsFromSelector(selector, kind, sns string, cli kubernetes.Interface, osCli osversioned.Interface, parsed map[string]bool) ([]crv1alpha1.ObjectReference, error) { //nolint:gocyclo
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this fail even with 30 cyclo limit?

Copy link
Contributor Author

@ankitjain235 ankitjain235 Oct 3, 2023

Choose a reason for hiding this comment

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

We are setting the limit to 20, that's where it's failing. It has complexity of 21.

pkg/kube/volume/volume.go Show resolved Hide resolved
Copy link
Contributor

@pavannd1 pavannd1 left a comment

Choose a reason for hiding this comment

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

Will re-review once we address the comments

Kanister automation moved this from Reviewer approved to Review Required Aug 22, 2023
@ankitjain235
Copy link
Contributor Author

@pavannd1 PTAL at the PR again.

Kanister automation moved this from Review Required to Reviewer approved Dec 1, 2023
@mergify mergify bot merged commit 5a1f552 into master Dec 4, 2023
15 checks passed
Kanister automation moved this from Reviewer approved to Done Dec 4, 2023
@mergify mergify bot deleted the enable-linters-1 branch December 4, 2023 05:57
@leuyentran leuyentran mentioned this pull request Dec 6, 2023
10 tasks
@ankitjain235 ankitjain235 linked an issue Dec 21, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

Enable gocritic and gocyclo linters
3 participants