Skip to content

Conversation

quantpoet
Copy link

What changed?

The new version of Go has been optimized, and variables do not need to be reassigned.

Why?

How did you test it?

Potential risks

Release notes

Documentation Changes

@davidporter-id-au
Copy link
Member

sorry, there was a few lint issues, do you mind addressing?

@quantpoet
Copy link
Author

sorry, there was a few lint issues, do you mind addressing?

lint...
simulation/replication/worker/cmd/main.go
  (118, 48)  https://revive.run/r#range-val-in-closure  loop variable domainName captured by func literal  

service/matching/tasklist/task_reader.go
  (140, 29)  https://revive.run/r#range-val-in-closure  loop variable g captured by func literal  


 ✖ 2 problems (2 errors) (0 warnings)
make[1]: *** [Makefile:397: .build/code-lint] Error 1
make: *** [Makefile:461: lint] Error 2

Hi, I checked the failed lint and found that it seems to be a bug in the lint tool.

As mentioned on the page ,

"Note: This rule is irrelevant for Go 1.22+."

Our project's Go version is above 1.22.

Directory: cmd/server, Go version: 1.23.0
Directory: common/archiver/gcloud, Go version: 1.23.0
Directory: internal/tools, Go version: 1.22
Directory: service/sharddistributor/store/etcd, Go version: 1.23.0

So, we can ignore the failure of this CI, and I will also file an issue for this tool.

@quantpoet
Copy link
Author

sorry, there was a few lint issues, do you mind addressing?

See mgechev/revive#1529

@dkrotx
Copy link
Member

dkrotx commented Sep 24, 2025

Let's update the linter, I guess it is outdated since the issue fixed in 1.5+ year ago.
Sorry, we can't ignore CI errors and land.

@quantpoet
Copy link
Author

Let's update the linter, I guess it is outdated since the issue fixed in 1.5+ year ago. Sorry, we can't ignore CI errors and land.

Yes, according to the author's response, the current version being used was released on 2023-05-16.

I have upgraded it in this PR; please review. #7273

@quantpoet
Copy link
Author

Let's update the linter, I guess it is outdated since the issue fixed in 1.5+ year ago. Sorry, we can't ignore CI errors and land.

Yes, the CI errors here can be ignored.

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.

3 participants