chore: make sure to run the CI on latest Go versions

[ccoVeille]

Also, add Go 1.18 to the list of Go versions to test against since it's still supported by go-redis.

[ccoVeille]

This first commit is a proof that there is a problem with minimal version of Go used in the project, and there is a need to validate the CI against Go 1.18

[ccoVeille]

Another way to consider this is to say that go-redis only supports Go 1.21 as minimal version and move forward

[ccoVeille]

Error: internal/pool/conn.go:72:19: undefined: atomic.Int64

atomic.Int64 and others were added to Go 1.19

https://pkg.go.dev/sync/atomic#Int64

At some points, there is a decision to take with the minimal version @ndyakov

[ndyakov]

@ccoVeille thank you for testing this. Although we were planing to change the go version in the next major release, this change (atomic.Int64) was already introduced so I assume at least bumping to 1.19 should be done. Let me know what you think.

[ccoVeille]

Right now, only maintnotif is broken with Go 1.18, the rest are test files.

But I'm surprised it's broken for months.

So either people are not using Go < 1.21, either people didn't update for months.

I didn't check for an issue mentioning the incompatibility any way, maybe there are some.

So yes, trying to use Go 1.19 could be a solution. I hope there would be no other fixes to do if I iterate.

I might open a new PR or simply an issue to clearly state the Go 1.19 problem.

Because it's not the same to fix the CI, and notice the minimal version is not the one expected.

I understand using Go 1.19 is a short term solution. In an ideal world, I would have replaced atomic.Int64 by the old syntax, and so keep the Go 1.18 compatibility.

But if no one reported, I don't feel an urge for a huge refactoring.

I wish the minimal version was set to Go 1.21, it is the case because slices and maps are used, but it wasn't intentional. So it should be considered as a bug.

TL; DR; let's try Go 1.19 and see if it works

[ccoVeille]

The code is using

Error: internal/routing/aggregator.go:486:9: a.res.And undefined (type "sync/atomic".Int64 has no field or method And)
Error: internal/routing/aggregator.go:488:9: a.res.And undefined (type "sync/atomic".Int64 has no field or method And)
Error: internal/routing/aggregator.go:570:9: a.res.Or undefined (type "sync/atomic".Int64 has no field or method Or)
Error: internal/routing/aggregator.go:572:9: a.res.Or undefined (type "sync/atomic".Int64 has no field or method Or)

https://pkg.go.dev/sync/atomic#Int64.Or

So the minimal version used by the code is Go 1.23

Which means it won't work as expected.

@ndyakov what is your call here ?

The code could be refactored maybe, but it's not fun at all.

I feel like it was broken since f711eb0 so with #3422 that was just merged

[ndyakov]

@ccoVeille the request response policy pr introduced https://github.com/uber-go/atomic, maybe they have a drop in replacements for older go versions ? This is still not released, so it is fine to resolve it (it is in 9.18.0-beta.1, but this is a beta, so it should be fine)

[ccoVeille]

@ccoVeille the request response policy pr introduced https://github.com/uber-go/atomic, maybe they have a drop in replacements for older go versions ?

I looked at it before posting my message earlier. uber/atomic provides the Int64 thing available in Go 1.19

But it doesn't provide the atomic.Int64.Or and atomic.Int64.And that the last PR is using.

This is still not released, so it is fine to resolve it (it is in 9.18.0-beta.1, but this is a beta, so it should be fine)

I didn't check if the issue is not anterior.

For record, I don't plan to work on fixing code that was added with the recent PRs.

I came to work with the logger and now I see oddities a bit everywhere 😅

I feel like go-redis v9 can be fixed to work with Go 1.18, but there are some refactoring to do to solve that.

[ndyakov]

@ccoVeille I am fine with making it work with 1.19 instead of 1.18, but still, let me see when I can fit the cycles to make the refactoring. Would you be okay with me merging this PR in a local branch so I can continue the refactoring?

[ccoVeille]

I feel like the Go 1.18 compatibility is easy to reach if you consider the only issues are about the atomic.Int64

So uber atomic could be used and it should work.

The issue you have is with the And and Or helper that the std lib brings. It would be a problem if it was a real bitwise thing, but I feel like it's about a basic Boolean operation.

Then about the fact to merge, you could. Sure.

But I could also suggest I could update the PR to make the test non-blocking on a failure with old Go versions.

It's a matter of configuration of the workflow.

Also, I could open a PR to merge the code changes I'm doing, without checking the workflow changes

Just tell me, or simply merge somewhere else.

[ndyakov]

since #3640 was merged, i think we can close this one. @ccoVeille close if you think the changes were sufficient and there is nothing to add with this PR