Skip to content

Comments

fix(lints): fix all lints and upgrade to go 1.24#163

Merged
dkropachev merged 1 commit intoscylladb:masterfrom
CodeLieutenant:fix/lints
Mar 3, 2025
Merged

fix(lints): fix all lints and upgrade to go 1.24#163
dkropachev merged 1 commit intoscylladb:masterfrom
CodeLieutenant:fix/lints

Conversation

@CodeLieutenant
Copy link
Contributor

@CodeLieutenant CodeLieutenant commented Mar 3, 2025

After introducing GolangCI Lint into the build pipeline, project CI started to fail, cause of the issues in the code base not being up to the lint standard. This addresses all the fixes to the code base as well as upgrade to Go 1.24 and it's new feature tool directive in go.mod, which simplifies the development (linting, formatting and struct optimizations)

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This pull request fixes lint issues across the code base and upgrades the project to Go 1.24, taking advantage of the new tool directive in go.mod.

  • Updated Go version in GitHub workflows and configuration files.
  • Refactored error formatting to use fmt.Errorf and adjusted import ordering.
  • Improved naming consistency and removed unused or redundant code constructs.

Reviewed Changes

File Description
modes.go Refactored import order, adjusted constructor signatures, and improved error formats.
.github/workflows/release.yml, go.yml Updated Go version from previous releases to 1.24.
pkg/results/result.go, thread_results.go, merged_result.go, thread_result.go Reordered fields and improved error message formatting.
random/random.go, random_test.go Revised usage of random package and distribution parsing for better lint compliance.
main.go Enhanced flag initialization and consistent naming in workload and mode functions.
pkg/workloads/workloads.go, workloads_test.go Corrected parameter naming and improved formatting consistency.

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

pkg/results/thread_result.go:83

  • There is a typo in the literal 'co-fixed-lantecy'. It should be corrected to 'co-fixed-latency' for consistency.
r.PartialResult.CoFixedLatency = NewHistogram(&globalResultConfiguration.latencyHistogramConfiguration, "co-fixed-lantecy")

modes.go:61

  • [nitpick] The ignored time offset parameter is retained in the function signature; consider removing it from both the signature and related comments if it is no longer needed.
func NewRateLimiter(maximumRate int, _ time.Duration) RateLimiter {

Signed-off-by: Dusan Malusev <dusan.malusev@scylladb.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR fixes lint issues throughout the codebase and upgrades the project to Go 1.24. Key changes include updating the Go version and dependencies (e.g. using "math/rand/v2"), reordering and cleaning up imports and code style in several files, and making minor refactoring adjustments to improve consistency and readability.

Reviewed Changes

File Description
modes.go Lint adjustments and minor refactoring of RateLimiter and logging
main.go Update of Go version and cleanup of code comments and formatting
.github/workflows/release.yml Upgrade to Go 1.24
pkg/results/result.go Reordering of histogram configuration fields
random/random.go Lint fixes, concurrency control adjustments, and refactoring adjustments in distribution parsing
pkg/results/merged_result.go Minor renaming and formatting changes
internal/version/version.go Update error handling and assignment style
pkg/workloads/workloads.go Refactoring function signatures and parameter naming changes
pkg/results/thread_result.go Minor clean-up and histogram naming adjustments

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

pkg/results/thread_result.go:126

  • The label 'co-fixed-lantecy' appears to be misspelled; it should probably be 'co-fixed-latency'.
r.PartialResult.CoFixedLatency = NewHistogram(&globalResultConfiguration.latencyHistogramConfiguration, "co-fixed-lantecy")

random/random.go:231

  • There is a duplicate return statement in the RandomInt64 function after the first return, which is unreachable and should be removed.
return generator.Int63n(maxValue)

@CodeLieutenant CodeLieutenant marked this pull request as ready for review March 3, 2025 17:58
@dkropachev dkropachev merged commit f513b31 into scylladb:master Mar 3, 2025
1 check passed
@CodeLieutenant CodeLieutenant deleted the fix/lints branch March 3, 2025 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants