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

Update golangci-lint to v1.55, fix lint errors #2539

Merged
merged 2 commits into from
Dec 15, 2023

Conversation

hairyhum
Copy link
Contributor

Change Overview

Currently golangci-lint is built with go version older than the go1.21 This can cause weird linter false-positives (e.g. https://github.com/kanisterio/kanister/actions/runs/7202258335/job/19620056020)

Updated version of golangci-lint in the build image Fixed linter warnings:

  • deprecated rand.Seed
  • non-sensical return in setupKopiaRepositoryServer

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

  • fixes #issue-number

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

Currently golangci-lint is built with go version older than the go1.21
This can cause weird linter false-positives (e.g. https://github.com/kanisterio/kanister/actions/runs/7202258335/job/19620056020)

Updated version of golangci-lint in the build image
Fixed linter warnings:
- deprecated rand.Seed
- non-sensical return in setupKopiaRepositoryServer
Copy link
Contributor

@julio-lopez julio-lopez left a comment

Choose a reason for hiding this comment

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

+1: LG 👍
See nit comment.

)

const characters = "abcdefghijklmnopqrstuvwxyz0123456789"

func init() {
rand.Seed(time.Now().UnixNano())

Copy link
Contributor

Choose a reason for hiding this comment

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

delete the init() function altogether, since it is empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

@hairyhum Please address this comment

Copy link
Contributor

Choose a reason for hiding this comment

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

To be addressed in #2566

@viveksinghggits
Copy link
Contributor

Hi @hairyhum ,
I raised a PR #2545 to update the base image's version in our build image. And after that the lint has been failing with signal killed.
I think updating the the golangci-lint would be a good idea for my PR as well. Let's merge this and then we can see why lint is complaining in my PR.

@hairyhum hairyhum added the kueue label Dec 15, 2023
@mergify mergify bot merged commit 4e9492c into master Dec 15, 2023
14 checks passed
@mergify mergify bot deleted the golint-bump-golangci-lint branch December 15, 2023 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants