Conversation
|
Looks good, I will leave it open a few days to see if anyone has comments. Cheers! |
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the codebase by replacing deprecated io/ioutil functions with their direct os package equivalents (introduced in Go 1.16) and updates iterator function signatures to use the standard iter.Seq type (introduced in Go 1.23). Additionally, it improves test hygiene by using t.TempDir() for temporary file management.
Key changes:
- Replaced
ioutil.ReadFile,ioutil.WriteFile,ioutil.TempDir, andioutil.TempFilewithos.ReadFile,os.WriteFile,os.MkdirTemp, andos.CreateTemp - Updated iterator function signatures from
func(func(T) bool)toiter.Seq[T] - Modified tests to use
t.TempDir()for automatic cleanup of temporary files
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| smat_generate_test.go | Replaced ioutil.WriteFile with os.WriteFile and removed deprecated import |
| serialization_test.go | Replaced ioutil.ReadFile with os.ReadFile, used t.TempDir() for test file management, and removed manual cleanup |
| serialization_frozen_test.go | Replaced all ioutil.ReadFile calls with os.ReadFile and removed deprecated import |
| roaring64/serialization_test.go | Replaced ioutil functions with os equivalents, used t.TempDir() for proper test isolation |
| roaring64/bsi64_test.go | Updated to use os.CreateTemp with t.TempDir() and removed manual cleanup |
| iter.go | Updated iterator return types to use iter.Seq[uint32] |
| roaring64/iter.go | Updated iterator return types to use iter.Seq[uint64] |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The main go.mod uses this as well now, so the CI should follow along.
|
FYI, I observed an odd test failure on macos, and noticed that the github workflow was still on go 1.22, so I've also upgraded the workflows to use go 1.24. |
I noticed some minor upgrades were possible now we've updated to Go 1.24.
This should be a pure refactoring, with no semantic changes, and no performance impact.