Run modernize. Deprecate Go 1.22 #1095
Conversation
📝 WalkthroughWalkthroughBump CI and module Go versions; widespread syntactic modernizations replacing Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
zstd/snappy.go (1)
416-419: Fix operator precedence in Snappy masked CRC computationThe Snappy framing spec masks the CRC as: ((c >> 15) | (c << 17)) + 0xa282ead8. The current expression
c>>15 | c<<17 + 0xa282ead8evaluates as(c>>15) | (c<<17 + const)due to+binding tighter than|, which can yield incorrect results.Apply parentheses to ensure the mask matches the spec:
func snappyCRC(b []byte) uint32 { c := crc32.Update(0, crcTable, b) - return c>>15 | c<<17 + 0xa282ead8 + // mask per Snappy framing format: ((c >> 15) | (c << 17)) + 0xa282ead8 + return ((c >> 15) | (c << 17)) + 0xa282ead8 }flate/stateless.go (1)
186-191: Type mismatch: range over int16 yields int; assigning to int16 field needs cast.
for i := range startAtproduces an int index.tableEntry.offsetis int16, sotableEntry{offset: i}will not compile on strict builders. Cast the index when storing.Apply this diff:
- for i := range startAt { - table[hashSL(cv)] = tableEntry{offset: i} + for i := range startAt { + table[hashSL(cv)] = tableEntry{offset: int16(i)} cv = (cv >> 8) | (uint32(src[i+4]) << 24) }Optional alternative (keeps i as int16, but deviates from your modernization goal):
- for i := range startAt { + for i := int16(0); i < startAt; i++ { table[hashSL(cv)] = tableEntry{offset: i} - cv = (cv >> 8) | (uint32(src[i+4]) << 24) + cv = (cv >> 8) | (uint32(src[i+4]) << 24) }s2/reader.go (1)
219-227: Bug: wrong variable used in error check after Seek in skippable.You’re checking
r.errinstead of the localerr. This can mask EOF/UEOF from Seek and lead to incorrect ErrCorrupt handling.Apply this diff:
- if err == io.ErrUnexpectedEOF || (r.err == io.EOF && !allowEOF) { + if err == io.ErrUnexpectedEOF || (err == io.EOF && !allowEOF) { r.err = ErrCorrupt return false }gzhttp/compress.go (1)
884-899: qvalue can be reset incorrectly when multiple parameters follow q=qvalue is reinitialized to DefaultQValue on every iteration of the loop. If the q= parameter isn’t the last one (e.g., "gzip; q=0.5; foo=bar"), the subsequent iteration resets qvalue to 1.0 and you’ll return the wrong quality. Initialize qvalue once before the loop and stop scanning after parsing q=.
Apply this diff:
func parseCoding(s string) (coding string, qvalue float64, err error) { // Avoid splitting if we can... if len(s) == 0 { return "", 0, errEmptyEncoding } if !strings.ContainsRune(s, ';') { coding = strings.ToLower(strings.TrimSpace(s)) if coding == "" { err = errEmptyEncoding } return coding, DefaultQValue, err } - for n, part := range strings.Split(s, ";") { - part = strings.TrimSpace(part) - qvalue = DefaultQValue + qvalue = DefaultQValue + for n, part := range strings.Split(s, ";") { + part = strings.TrimSpace(part) if n == 0 { coding = strings.ToLower(part) - } else if after, ok := strings.CutPrefix(part, "q="); ok { + } else if after, ok := strings.CutPrefix(part, "q="); ok { qvalue, err = strconv.ParseFloat(after, 64) if qvalue < 0.0 { qvalue = 0.0 } else if qvalue > 1.0 { qvalue = 1.0 } + // Done: ignore any subsequent params after q= + break } }This keeps the existing forgiving behavior while ensuring q= is honored regardless of parameter order. I can add targeted tests for mixed-parameter orders if helpful.
zstd/dict.go (1)
427-434: Division-by-zero risk when litTotal == 0avgSize := min(litTotal, huff0.BlockSizeMax/2) can be 0 if litTotal is 0, and then div := max(litTotal/avgSize, 1) will divide by zero. While litTotal is usually > 0, it’s not guaranteed by the preceding checks (seqs > 0 doesn’t imply literals > 0).
Guard avgSize (or the divisor) to be at least 1 before the division.
Apply:
- avgSize := min(litTotal, huff0.BlockSizeMax/2) + avgSize := min(litTotal, huff0.BlockSizeMax/2) + if avgSize < 1 { + avgSize = 1 + } huffBuff := make([]byte, 0, avgSize) // Target size - div := max(litTotal/avgSize, 1) + div := max(litTotal/avgSize, 1)Alternatively, fold the guard into the expression:
- div := max(litTotal/avgSize, 1) + div := max(litTotal/max(avgSize, 1), 1)zstd/enc_dfast.go (1)
629-641: Fix incorrect short-table hash in EncodeNoHist’s “Check offset 2” loopIn zstd/enc_dfast.go’s EncodeNoHist (around line 621), the code currently does:
// Store this, since we have it. nextHashS := hashLen(cv1>>8, dFastShortTableBits, dFastShortLen) nextHashL := hashLen(cv, dFastLongTableBits, dFastLongLen) … e.table[nextHashS] = entryHere
cv1holds the previous match’s last-8-bytes value (shifted once), not the current window ats. The main search loop and the non-NoHist path both hash the currentcvfor both tables, so usingcv1>>8here desynchronizes the short-table update and can degrade match chaining. Change to:- nextHashS := hashLen(cv1>>8, dFastShortTableBits, dFastShortLen) + nextHashS := hashLen(cv, dFastShortTableBits, dFastShortLen)This aligns the short-table update with the long-table (and with the Encode path), ensuring future lookups use the correct window bytes.
♻️ Duplicate comments (4)
zstd/enc_dfast.go (4)
266-271: Same tMin consolidation applies here. LGTM.
459-464: Same tMin consolidation applies here. LGTM.
800-806: Same tMin consolidation applies here. LGTM.
915-920: Same tMin consolidation applies here. LGTM.
🧹 Nitpick comments (29)
go.mod (1)
3-3: Go version bump to 1.23 looks good and matches the PR intent.This enables modernize-driven syntax (e.g., builtin min/max, range-over-integers is ≥1.22) while deprecating Go 1.22. CI/release files in this PR also target ≥1.23, so the ecosystem stays aligned.
Optionally consider adding a
toolchaindirective in go.mod to pin the build toolchain used by contributors to the same major.minor you release with (currently 1.25.x) to reduce "works on my machine" drift. No blocker.zstd/zstd.go (1)
101-111: Vararg type switch toanyis a no-op semantically.
anyis an alias ofinterface{}since Go 1.18; call sites and logging behavior are unchanged. Local namesprintln/printfcontinue to guard on the same debug flags.If you ever grep for accidental uses of the predeclared
println, consider renaming these helpers (e.g.,dbgPrintln/dbgPrintf) to avoid confusion. No action required for this PR.s2/writer.go (1)
50-52: Switching sync.Pool.New return type to any is correct and future-proofNo behavioral change; aligns with Go's built-in alias and repository-wide modernization. Calls to Get().([]byte) continue to work.
Optional micro-improvement to reduce needless initial length:
- w2.buffers.New = func() any { - return make([]byte, w2.obufLen) - } + w2.buffers.New = func() any { + // Keep capacity for obufLen but start with length 0; callers slice to needed length. + return make([]byte, 0, w2.obufLen) + }huff0/bitwriter.go (1)
88-93: Add explicit uint cast for shift countThe range-over-integers refactor is safe under Go 1.23 (go.mod specifies
go 1.23), but adding an explicituintcast on the shift count will future-proof against potential type changes tonbBytes.• File:
huff0/bitwriter.go, lines 88–93
• Verifiedgo.modhas ago 1.23directive at line 3, meeting the ≥ 1.22 requirement.Suggested diff:
-for i := range nbBytes { - b.out = append(b.out, byte(b.bitContainer>>(i*8))) -} +for i := range nbBytes { + b.out = append(b.out, byte(b.bitContainer>>(uint(i)*8))) +}flate/dict_decoder.go (1)
103-111: Use of min() keeps behavior; note on overflow (pre-existing)
endPos := min(dstPos+length, len(dd.hist))is equivalent to the prior clamp and reads cleaner. Note that the addition was already susceptible to theoreticalintoverflow before this change; your refactor neither introduces nor fixes that. If you ever want to harden for extreme sizes on 32-bit platforms, we can switch to a checked add.zstd/decoder.go (1)
373-383: Guard against potential int overflow when doubling input length (32-bit edge).The new min(len(input)*2, 1<<20) reads well, but len(input)*2 can overflow on 32-bit builds when len(input) > math.MaxInt/2. If that happens, size becomes negative, the subsequent uint64(size) comparison will promote it to a very large value, and the allocation may be bumped up to maxDecodedSize, unintentionally ignoring the 1 MiB cap. Not a new risk, but easy to harden here.
Apply this diff to avoid overflow while preserving intent:
- size := min( - // Cap to 1 MB. - len(input)*2, 1<<20) + // Default to 2x input, capped at 1 MiB, without risking int overflow on 32-bit. + var size int + if len(input) >= 1<<19 { // 2*len(input) would exceed 1 MiB or overflow on 32-bit. + size = 1 << 20 + } else { + size = len(input) << 1 + }zstd/bitwriter.go (1)
91-96: flushAlign modernization looks good; add an explicit unsigned cast in the shift (minor).
- Using range over an integer and clearing bitContainer are both good improvements.
- To future-proof the shift if nbBytes’ type ever changes (e.g., to a signed int), cast the shift count to uint to avoid compile errors.
- for i := range nbBytes { - b.out = append(b.out, byte(b.bitContainer>>(i*8))) + for i := range nbBytes { + b.out = append(b.out, byte(b.bitContainer>>(uint(i)*8))) }zstd/framedec.go (1)
239-248: WindowSize clamp via max() for SingleSegment is fine.For SingleSegment frames without an explicit Window_Descriptor, using WindowSize = max(FrameContentSize, MinWindowSize) matches the spec intent and prior behavior. Follow-up: if there’s any chance FrameContentSize could be fcsUnknown here (it shouldn’t per spec), consider guarding it explicitly to avoid an unintended large window default.
huff0/decompress_amd64.go (1)
4-6: Spelling nit in comment.“thir main loops” → “their main loops”.
Apply this diff:
-// and Decoder.Decompress1X that use an asm implementation of thir main loops. +// and Decoder.Decompress1X that use an asm implementation of their main loops.s2/reader.go (1)
1049-1059: Range-over-integer loop is fine; consider a named constant for readability.
for range 10is correct for 10 retries. Consider a named const (e.g.,const maxByteReadRetries = 10) to make the intent self-documenting.- for range 10 { + const maxByteReadRetries = 10 + for range maxByteReadRetries { n, err := r.Read(tmp[:])huff0/compress.go (2)
309-346: Potential table-header truncation in compress4Xp: preserve existing s.Out and index with an offset.Unlike compress4X (which appends six placeholder bytes after any existing table), compress4Xp currently does
s.Out = s.Out[:6]. If compress4Xp is ever enabled in Compress4X (the if false guard lifted), this will truncate any table already written by compress(), corrupting the output header. Recommend mirroring the compress4X approach by appending the six bytes and indexing lengths with an offset.Apply this diff:
@@ - // Add placeholder for output length - s.Out = s.Out[:6] + // Add placeholder for output length (preserve any already written table bytes) + offsetIdx := len(s.Out) + s.Out = append(s.Out, sixZeros[:]...) @@ - if i < 3 { - // Last length is not written. - s.Out[i*2] = byte(len(o)) - s.Out[i*2+1] = byte(len(o) >> 8) - } + if i < 3 { + // Last length is not written. + s.Out[i*2+offsetIdx] = byte(len(o)) + s.Out[i*2+offsetIdx+1] = byte(len(o) >> 8) + }This future-proofs compress4Xp without affecting current behavior.
329-344: Result collation loop OK; just remember offset if the header change above is applied.As-is, writing sizes at s.Out[0:6] and then appending the 4 payloads is consistent. If you adopt the offset-based header write suggested earlier, update the indexes accordingly (i.e., add offsetIdx).
zstd/fse_encoder.go (1)
147-159: for range v preserves semantics, including v <= 0 yielding zero iterations.The previous loop ran zero times when v <= 0; for range v maintains that behavior. Since v is int16, the loop count remains bounded by table size. If you want to make intent explicit to readers, consider casting for clarity:
- for range v { + for range int(v) { tableSymbol[position] = symbol ... }zstd/enc_better.go (1)
193-193: tMin clamping via max() is a nice simplification; semantics unchanged.These replacements read clearer and avoid the extra branch. Minor nit: if max is a generic helper, consider using a typed zero (int32(0)) to make intent explicit to readers and silence overzealous linters.
Also applies to: 252-252, 477-477, 713-713, 774-774, 993-993
gzhttp/asserts_test.go (3)
26-33: Make failure messages more actionable in assertNil.Including the received value helps debug failures. Optional tweak shown below.
-func assertNil(t testing.TB, object any) { - if isNil(object) { - return - } - t.Helper() - t.Fatalf("Expected value to be nil.") -} +func assertNil(t testing.TB, object any) { + if isNil(object) { + return + } + t.Helper() + t.Fatalf("Expected value to be nil, got %#v.", object) +}
34-40: assertNotNil: Mirror message to show actual value.Minor ergonomics: include the offending value.
-func assertNotNil(t testing.TB, object any) { - if !isNil(object) { - return - } - t.Helper() - t.Fatalf("Expected value not to be nil.") -} +func assertNotNil(t testing.TB, object any) { + if !isNil(object) { + return + } + t.Helper() + t.Fatalf("Expected value not to be nil, got %#v.", object) +}
66-71: Prefer iterating values directly in containsKind.Idiomatic and slightly clearer.
-func containsKind(kinds []reflect.Kind, kind reflect.Kind) bool { - for i := range kinds { - if kind == kinds[i] { - return true - } - } - return false -} +func containsKind(kinds []reflect.Kind, kind reflect.Kind) bool { + for _, k := range kinds { + if kind == k { + return true + } + } + return false +}snappy/snappy_test.go (1)
601-612: Replace magic number 1e5 with a typed constant and reuse in indexes.You already cast the loop bound to
int(1e5). Using a single typed constant avoids repeated casts and clarifies intent, and also fixes the mixed literal in indexing (src[1e5*i+j]) to be consistently typed.Apply within this hunk:
- for i := range 10 { + for i := range 10 { if i%2 == 0 { - for j := range int(1e5) { - src[1e5*i+j] = uint8(rng.Intn(256)) + for j := range block { + src[block*i+j] = uint8(rng.Intn(256)) } } else { - for j := range int(1e5) { - src[1e5*i+j] = uint8(i) + for j := range block { + src[block*i+j] = uint8(i) } } }Add this just above the loop (outside the changed hunk):
const block = 100000 // 1e5, typed as int for indexing and rangezstd/seqdec_amd64_test.go (1)
135-136: Nice: fmt.Appendf avoids an intermediate string; consider checking the write error.Minor nit: capture and handle the
Writeerror to avoid hiding I/O failures in rare cases.Apply this diff:
- w.Write(fmt.Appendf(nil, "%d,%d,%d\n", s.prevOffset[0], s.prevOffset[1], s.prevOffset[2])) + if _, err := w.Write(fmt.Appendf(nil, "%d,%d,%d\n", s.prevOffset[0], s.prevOffset[1], s.prevOffset[2])); err != nil { + fatalIf(err) + }internal/snapref/decode.go (1)
212-217: Go 1.22+ integer-range is fine here; optionally simplify with bytes.Equal for clarity.Your use of
for i := range len(magicBody)is valid with the updated toolchain. For readability and a tiny simplification, consider a direct slice compare.Here’s a possible tweak (adds one import):
@@ -import ( +import ( "encoding/binary" "errors" "io" + "bytes" ) @@ - for i := range len(magicBody) { - if r.buf[i] != magicBody[i] { - r.err = ErrCorrupt - return r.err - } - } + if !bytes.Equal(r.buf[:len(magicBody)], magicBody) { + r.err = ErrCorrupt + return r.err + }flate/inflate.go (1)
779-781: Range-over-constant is fine; optional consistency nit.
for i := range 144is correct and concise. If you want uniform style, you could optionally convert the subsequent three literal loops (144–256, 256–280, 280–288) to the same pattern for consistency, but it’s not required.flate/writer_test.go (1)
377-382: Large-iteration range expression: verify integer sizing on 32-bit.
for range (math.MaxUint32 - bufferReset) / maxMatchOffsetcan be very large and the expression mixes typed/untitled ints anduint32. It compiles today, but as a guard against 32-bit builds, consider an explicit cast tointon the final quotient to ensure the iteration count is inintspace and avoid subtle type promotions.Apply if desired:
- for range (math.MaxUint32 - bufferReset) / maxMatchOffset { + for range int((math.MaxUint32 - uint32(bufferReset)) / uint32(maxMatchOffset)) {zstd/blockdec.go (1)
556-623: Range-over-uint(3) works; small readability nit.
for i := range uint(3)is correct. Sinceiis immediately converted totableIndex(i)and used in arithmetic(6 - i*2), a plainfor i := range 3(defaultingito int) would avoid implicit unsigned arithmetic for the shift count and may be marginally clearer. Not required.Apply if you prefer:
- for i := range uint(3) { + for i := range 3 {zstd/enc_fast.go (3)
146-151: Boundary clamp via max(): nice simplification; confirm helper’s type covers int32The switch to sMin/tMin := max(s-e.maxMatchOff, 0) is cleaner and reduces branchiness. Please confirm the max helper is generic (or otherwise typed) to accept int32 operands; otherwise an implicit conversion could sneak in or fail compilation on some platforms. If max is int-typed only, consider casting 0 to int32 to keep everything in int32 land.
Example if needed:
- sMin := max(s-e.maxMatchOff, 0) + sMin := max(s-e.maxMatchOff, int32(0))Also applies to: 223-229
384-389: Same clamp improvement in EncodeNoHist; type safety applies here as wellThe EncodeNoHist path mirrors the same clamp. The same note about max’s type parameters and literal 0 applies here.
Also applies to: 463-469
646-651: Consistent lower-bound via max() in dict path: good; keep an eye on helper’s genericsThe unified tMin/sMin clamp is consistent across paths and looks good. As above, ensure max supports int32 to avoid silent widening.
Also applies to: 723-729
gzhttp/compress_test.go (1)
978-985: Bound clamp with min(): fine; ensure min is available in package scopeUsing toWrite := min(100, len(tb)) is concise. Just verify gzhttp defines a generic/int min helper (and that tests import it from the same package). Otherwise this will not compile.
If needed, add a local helper in gzhttp:
+// min returns the smaller of a and b. +func min[T ~int | ~int32 | ~int64](a, b T) T { + if a < b { return a } + return b +}zstd/dict.go (1)
197-211: Logging helpers switched to any: consistent with modern GoChanging println/printf/print closures to accept ...any aligns with current idioms. Minor note: these names shadow predeclared print/println—fine in local scope, but some linters may complain. If that happens, consider renaming to dbgPrint/dbgPrintf/dbgPrintln.
zstd/decoder_test.go (1)
117-123: Clamp start offset with max(): good; prefer explicit int64 for readabilityThe non-negative clamp is correct. For clarity and to avoid implicit constant typing, consider making both operands explicitly int64.
- start := max(int64(cHash)/4*blockSize-sizeBack, 0) + start := max(int64(cHash/4*blockSize)-int64(sizeBack), int64(0))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (65)
.github/workflows/go.yml(6 hunks).github/workflows/release.yml(1 hunks)dict/builder.go(2 hunks)flate/deflate.go(5 hunks)flate/deflate_test.go(5 hunks)flate/dict_decoder.go(1 hunks)flate/huffman_bit_writer.go(3 hunks)flate/huffman_code.go(1 hunks)flate/inflate.go(2 hunks)flate/level5.go(1 hunks)flate/stateless.go(2 hunks)flate/token_test.go(1 hunks)flate/writer_test.go(4 hunks)fse/bitwriter.go(1 hunks)fse/compress.go(1 hunks)go.mod(1 hunks)gzhttp/asserts_test.go(3 hunks)gzhttp/compress.go(3 hunks)gzhttp/compress_test.go(13 hunks)gzhttp/writer/gzkp/gzkp.go(1 hunks)gzhttp/writer/gzstd/stdlib.go(1 hunks)gzip/gunzip_test.go(1 hunks)gzip/gzip_test.go(2 hunks)huff0/bitwriter.go(1 hunks)huff0/compress.go(3 hunks)huff0/compress_test.go(2 hunks)huff0/decompress.go(4 hunks)huff0/decompress_amd64.go(2 hunks)huff0/huff0.go(2 hunks)internal/lz4ref/block.go(1 hunks)internal/snapref/decode.go(1 hunks)s2/encode_all.go(1 hunks)s2/encode_best.go(1 hunks)s2/encode_better.go(1 hunks)s2/reader.go(1 hunks)s2/s2_test.go(5 hunks)s2/writer.go(1 hunks)snappy/snappy_test.go(2 hunks)snappy/xerial/xerial_test.go(2 hunks)zip/reader_test.go(3 hunks)zip/writer_test.go(1 hunks)zip/zip_test.go(6 hunks)zlib/writer_test.go(1 hunks)zstd/bitwriter.go(1 hunks)zstd/blockdec.go(2 hunks)zstd/decoder.go(1 hunks)zstd/decoder_test.go(6 hunks)zstd/dict.go(4 hunks)zstd/dict_test.go(0 hunks)zstd/enc_base.go(1 hunks)zstd/enc_best.go(4 hunks)zstd/enc_better.go(6 hunks)zstd/enc_dfast.go(6 hunks)zstd/enc_fast.go(6 hunks)zstd/framedec.go(1 hunks)zstd/fse_encoder.go(1 hunks)zstd/internal/xxhash/xxhash_test.go(1 hunks)zstd/seqdec.go(1 hunks)zstd/seqdec_amd64.go(2 hunks)zstd/seqdec_amd64_test.go(1 hunks)zstd/seqdec_test.go(3 hunks)zstd/snappy.go(1 hunks)zstd/zip.go(1 hunks)zstd/zstd.go(1 hunks)zstd/zstd_test.go(3 hunks)
💤 Files with no reviewable changes (1)
- zstd/dict_test.go
🧰 Additional context used
🧬 Code graph analysis (16)
gzhttp/writer/gzkp/gzkp.go (2)
zlib/writer.go (2)
NewWriterLevel(56-58)w(29-38)gzip/gzip.go (3)
NewWriter(50-53)NewWriterLevel(61-68)z(70-83)
flate/level5.go (1)
flate/snappy.go (1)
e(839-894)
s2/encode_better.go (1)
s2/dict.go (1)
MaxDictSrcOffset(21-21)
s2/encode_all.go (1)
s2/dict.go (1)
MaxDictSrcOffset(21-21)
gzip/gunzip_test.go (1)
flate/inflate_test.go (1)
TestWriteTo(233-282)
zip/writer_test.go (2)
compressible_test.go (1)
BenchmarkCompressAllocations(243-273)fse/fse_test.go (1)
BenchmarkCompress(154-179)
zstd/snappy.go (1)
snappy/decode.go (3)
r(86-94)r(99-105)r(118-237)
internal/snapref/decode.go (1)
snappy/decode.go (3)
r(86-94)r(118-237)r(99-105)
s2/encode_best.go (1)
s2/dict.go (1)
MaxDictSrcOffset(21-21)
flate/stateless.go (1)
flate/snappy.go (1)
Encode(23-26)
huff0/compress_test.go (2)
fse/fse_test.go (2)
BenchmarkCompress(154-179)TestCompress(67-86)compressible_test.go (1)
BenchmarkCompressAllocations(243-273)
zstd/enc_best.go (1)
compressible.go (1)
ShannonEntropyBits(68-85)
flate/dict_decoder.go (1)
flate/dict_decoder_test.go (1)
dist(44-47)
snappy/snappy_test.go (1)
flate/asm_test.go (1)
TestHistogram(146-183)
zstd/dict.go (1)
huff0/huff0.go (1)
BlockSizeMax(27-27)
gzhttp/asserts_test.go (2)
zip/struct.go (1)
interface{}(113-113)snappy/golden_test.go (1)
i(24-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: build (1.25.x, macos-latest)
- GitHub Check: build (1.25.x, ubuntu-latest)
- GitHub Check: build (1.24.x, windows-latest)
- GitHub Check: build (1.24.x, macos-latest)
- GitHub Check: build (1.25.x, windows-latest)
- GitHub Check: build (1.23.x, windows-latest)
- GitHub Check: build (1.24.x, ubuntu-latest)
- GitHub Check: fuzz-s2 (nounsafe)
- GitHub Check: build (1.23.x, ubuntu-latest)
- GitHub Check: fuzz-other ("noasm,nounsafe")
- GitHub Check: fuzz-other (nounsafe)
- GitHub Check: build (1.23.x, macos-latest)
- GitHub Check: fuzz-zstd (nounsafe)
- GitHub Check: fuzz-zstd ("noasm,nounsafe")
- GitHub Check: fuzz-s2 ("noasm,nounsafe")
🔇 Additional comments (86)
gzip/gunzip_test.go (1)
522-522: Modernized fixed-count loop is correct and clearer.
for range 2runs exactly twice and you don’t use the loop index, matching the previous intent. Withgo 1.23in go.mod, this syntax is supported..github/workflows/release.yml (2)
24-24: Upgrade to Go 1.25.x for releases aligns with CI matrix and deprecation.Good to see the action pinned to a commit. This should keep release builds on a recent toolchain while the module’s
go 1.23keeps source compatibility broad.If you want to double-check workflow alignment, ensure
.github/workflows/go.ymlalso includes1.25.xin its matrix and any pinned steps don’t implicitly pull an older Go.
29-29: GoReleaser bump to 2.11.2 is fine; no config changes here.Given major/minor bumps, just confirm your
.goreleaser.ymldoesn’t rely on deprecated fields removed in v2.x. Nothing in this file suggests a mismatch.If desired, run a dry-run locally in CI with
goreleaser release --clean --skip=publishto surface config warnings.zstd/seqdec.go (1)
234-234: ✅ Verified no localmindefinitions; builtinminis safe to use
I ran a search for anyminfunctions, variables, or constants in thezstdpackage and found none. The builtinmincannot be shadowed here, so replacing the custom implementation with Go’sminis valid and reduces unnecessary branches. Approving these changes.zlib/writer_test.go (1)
84-89: Range over slice indices is idiomatic and equivalentfor i := range b0 eliminates an extra len call and matches the earlier length check. Semantics unchanged for the byte-by-byte comparison.
zstd/internal/xxhash/xxhash_test.go (1)
118-133: Modern counted loop is fine herefor i := range 64 reads cleanly and preserves the loop bounds; no behavior change in the assertions.
flate/level5.go (1)
680-684: min(...) clamp simplifies bounds without changing behaviorReplacing the manual “if s1 > len(src)” with min(int(s)+maxMatchLength-4, len(src)) is equivalent and reduces branching. Nice tidy-up.
zstd/snappy.go (1)
257-266: Modernized loop with range-over-integers looks correctThe new
for i := range len(snappyMagicBody)iterates 0..len-1 and preserves the previous semantics. Given go.mod targets ≥1.23 in this PR, this is fine.fse/bitwriter.go (1)
145-151: Range-over-integers refactor is sound here
nbBytesis auint8; ranging over it yieldsiasuint8, which keeps the shift count typed as unsigned inb.bitContainer>>(i*8). Semantics and bounds remain identical to the oldfor i := uint8(0); i < nbBytes; i++loop.zip/writer_test.go (1)
632-642: Test loop modernized appropriately
for range 3is a concise drop-in for a fixed trip count and keeps the benchmark behavior unchanged. Good use in test code.zstd/enc_base.go (1)
43-48: Window size clamp via max() is equivalent and clearer
max(int32(1)<<uint(bits.Len(uint(size))), 1024)preserves the previous minimum window constraint while simplifying the logic. Return type remainsint32. Looks good.s2/encode_all.go (1)
906-907: sLimit clamp simplification is correct and equivalent.Replacing the two-step clamp with min(len(src)-inputMargin, MaxDictSrcOffset-maxAhead) keeps behavior unchanged and improves readability.
flate/huffman_code.go (1)
94-116: Range-over-integers usage is clear and correct for Go ≥1.22.Iterating with for ch = range uint16(literalCount) matches the declared ch type, avoids casts, and preserves the original logic. No issues.
s2/encode_better.go (1)
917-917: sLimit clamp rewritten with min(): semantics preserved.This mirrors the pattern used elsewhere and keeps the MaxDictSrcOffset/maxAhead bound intact. Looks good.
s2/encode_best.go (1)
45-46: Clamp simplification with min() is correct and preserves bounds.This consolidates the two-step clamp of sLimitDict into a single min(), keeping the search safely within both src and dictionary bounds. No behavioral changes observed.
flate/stateless.go (1)
59-62: sync.Pool New uses any — matches modern Go and stdlib style.The pool continues to return *huffmanBitWriter, so the Get().(*huffmanBitWriter) assertion remains valid. Good modernization.
huff0/decompress_amd64.go (2)
61-71: Modernized fixed-trip loop is OK.
for i := range 3preserves the 3 iterations used to decode the jump table and reads cleanly.
112-113: endsAt computation via min() is equivalent and clearer.This unifies the clamp logic and avoids off-by-one/OOB risks when filling tails.
huff0/compress.go (2)
279-299: Modern range-over-integers looks good here.Semantics are preserved: i iterates 0..3, and the jump-table writes (only for i < 3) match the old counting loop. No off-by-one or bounds risks introduced.
315-327: Concurrency loop is correct; closure capture is safe.Good use of passing i as a parameter to the goroutine avoids the classic loop-var capture bug. Declaring toDo inside the loop also ensures each goroutine sees the intended slice. No race on src since it’s read-only thereafter.
huff0/compress_test.go (2)
403-411: Adopted range-over-integers reads well and matches prior behavior.for i := range 255 iterates 0..254 as before. The testing.Short gate remains correct. Nice cleanup.
437-445: Same here—range-over-integers is fine.Semantics are unchanged; no off-by-one or type issues introduced.
flate/token_test.go (1)
10-11: Switch to any is correct and aligns with modern Go.Matches stdlib’s testing.T Fatal signature; no impact on callers.
gzhttp/writer/gzstd/stdlib.go (1)
38-44: sync.Pool New returning any is idiomatic now.Type assertion at Get sites remains to *gzip.Writer, so behavior is unchanged.
gzhttp/writer/gzkp/gzkp.go (1)
38-45: Switch to any in sync.Pool constructor is correct and future-proof.This aligns with modern Go usage and has no behavioral impact. The type assertion at Get() remains safe.
gzip/gzip_test.go (2)
249-251: Range-over-int conversion is valid in Go ≥1.22 and preserves behavior.for j := range i iterates 0..i-1 and the slice copy uses j correctly. Looks good.
327-329: Same here: range-over-int is fine and keeps semantics intact.The repeated copy logic remains correct.
zstd/zip.go (1)
22-28: any in sync.Pool.New is the right modernization.No functional change; pool.Get type assertion remains correct. LGTM.
snappy/xerial/xerial_test.go (1)
70-75: Loop modernization is fine.Iterating with for i := range xerialHeader is idiomatic and equivalent to the previous index loop here.
internal/lz4ref/block.go (1)
80-81: Use of any in sync.Pool initializer is correct and future-proof.Switching to
func() anyaligns with modern Go and the stdlib signature ofsync.Pool.New. The downcast to*Compressorat call sites remains valid.gzhttp/asserts_test.go (3)
12-17: Modernizing test helpers to any is fine.Changing
interface{}toanyimproves readability without altering behavior.
19-24: assertNotEqual: OK.Signature and logic remain equivalent; message is clear.
43-62: isNil: Logic is sound.Nil handling for typed and untyped nils is correct; guarding with
object == nilavoidsreflect.ValueOf(nil)pitfalls.snappy/snappy_test.go (1)
98-104: Range-over-integers for fixed loop count looks good.
for i := range 32is concise, clear, and requires Go 1.22+, which is covered by the updated CI/matrix..github/workflows/go.yml (1)
15-16: CI matrix and go.mod minimum version are alignedThe go.mod file specifies a
go 1.23directive, which matches the CI matrix of Go 1.23.x–1.25.x. No further updates are needed.fse/compress.go (1)
399-405: Range-over-integer refactor looks correct; semantics preserved for negative counts.Using
for range v(wherevisint16) preserves the previous behavior: whenv <= 0the loop runs zero times, which matches the priorfor nbOccurrences := int16(0); nbOccurrences < v; nbOccurrences++ { ... }. This relies on Go 1.22+ integer-range semantics, which aligns with the PR’s minimum toolchain. No functional change detected.dict/builder.go (1)
106-115: Switching variadic debug helpers to ...any is appropriate.Modernizes away from ...interface{} without behavior change.
huff0/huff0.go (2)
204-209: Range-over-integer loop matches prior bounds and intent.
for n := range maxSymbolValueiterates 0..(maxSymbolValue-1), same as the previousn < maxSymbolValue. Indexing intoc/huffWeightwithintis fine. No functional change.
274-279: Same here: safe refactor with unchanged behavior.Iteration count and side effects (
hist/huffWeightpopulation) remain identical.flate/inflate.go (1)
488-497: Good use of integer-range; semantics preserved.Switching to
for i := range nclenis equivalent here and removes the explicit counter. This relies on Go 1.22+; given the PR deprecates 1.22 and targets 1.23+ this is fine.flate/writer_test.go (3)
283-324: Modernized loop reads clean; behavior unchanged.Using
for l := range 10is equivalent to the oldfor l := 0; l < 10; l++and simplifies the test without altering logic.
337-392: Nice capture fix for subtests.Keeping
l := lbeforet.Runavoids the classic loop-variable capture pitfall with subtests andt.Parallel(). Good call.
367-371: Range-based “burn-in” loop is fine.Replacing
for i := 0; i < 50; i++withfor range 50keeps intent clear and avoids an unused variable.huff0/decompress.go (3)
630-639: Range-over-3 initializer: OK.
for i := range 3is equivalent to the previous fixed-trip loop and keeps initialization tight. Index mathi*2remains correct for the length field pairs.
865-874: Second 3-stream init: OK.Same change as above; no functional differences introduced.
1035-1036: Consistency: min clamp mirrored in Exactly path.Mirroring the clamp keeps both 8-bit paths aligned. Good.
zstd/zstd_test.go (3)
103-105: Tightened benchmark/test loop.
for range 100conveys intent and removes an unused loop variable. Behavior is identical.
133-147: Flush path loop modernization looks good.Range-16 is appropriate here and removes unused index; no change to semantics.
159-169: No-flush path loop modernization looks good.Same as above; clean and equivalent.
zstd/blockdec.go (1)
57-63: Use of any in sync.Pool.New matches modern style.Switching
interface{}toanyis idiomatic and consistent with current stdlib signatures.gzhttp/compress.go (2)
106-109: Simplify wantBuf with max() — good modernizationUsing the built-in max consolidates the initial buffer sizing logic cleanly. Semantics match the previous code path and keep jitterBuffer override intact.
455-455: sync.Pool New uses any — consistent with Go 1.18+ idiomsSwitching from interface{} to any is purely cosmetic and consistent with the rest of the PR. No behavior change.
zstd/seqdec_test.go (2)
106-108: Range-over-integers is correct and clearer herefor j := range 3 eliminates loop boilerplate and reads well with fixed-size [3]int arrays. This also avoids an unused-loop-index warning.
Also applies to: 120-127
282-283: Use fmt.Appendf to avoid string allocation — nice micro-optimizationfmt.Appendf(nil, ...) avoids the temporary string and slice conversion previously done with Sprintf and []byte(...). Good change for hot test paths generating lots of output.
zip/reader_test.go (3)
663-671: Numeric range loop is equivalent and conciseReplacing for i := 0; i < 5; i++ with for range 5 keeps the semantics, simplifies the code, and avoids an unused i. Concurrency logic with n and done remains unchanged.
1053-1066: Outer decompression loop: range 2 is finefor range 2 preserves the two-iteration behavior while dropping an unused index. The inner logic still reassigns b by decoding twice as intended.
1460-1465: Writer empty-file creation: range 5 is appropriateThe semantics of creating five empty entries are preserved. Assertions below still expect 5 files.
zstd/enc_best.go (1)
159-164: Clamp and bound rewrites via min/max — good, but keep the bounds rationale clear
- bitsPerByte now guarantees a floor of 1 bit/byte (scaled), preventing negative-gain matches from skewing estimates.
- tMin := max(s-e.maxMatchOff, 0) is equivalent and easier to read.
- The end := min(...) guards maintain safe load6432 bounds:
- sLimit is len(src)-inputMargin, so using sLimit+4 ensures index0 < len(src)-8 before 8-byte loads.
- Using sLimit-4 in the non-repeat path keeps prior conservative indexing.
Overall no semantic changes, with reduced branching in hot loops.
Also applies to: 237-241, 381-391, 439-450
s2/s2_test.go (4)
65-67: Iterating up to maxBlockSize with range over int64 — OKUsing for i := range int64(maxBlockSize) fits the int64 test inputs without casts and keeps testSet types consistent.
165-170: Inner small-copy loop modernizedfor range 32 is equivalent and removes an unused index declaration; the string construction and roundtrip remain unchanged.
753-762: Framing format generators: numeric ranges read cleanlySwitching the outer 10-iteration loop and inner 1e5-iteration loops to range maintains behavior and types (explicit int(...) where needed). Good readability win.
Also applies to: 789-798
1676-1681: EstimateBlockSize smoke test: simplified loopfor range 100 expresses intent clearly and leaves input growth unaffected.
zstd/enc_fast.go (1)
628-631: Marking shards dirty on table updates is the right callAdding e.markShardDirty(nextHash/nextHash2) ensures Reset can refresh only touched shards. This reduces unnecessary copying and keeps dict-table coherence. Nice improvement.
Also applies to: 757-760
gzhttp/compress_test.go (1)
1064-1083: Approve range-over-integer idiom
Confirmed thatgo.modspecifies Go 1.23 (see line 3), sofor i := range Nis fully supported and idiomatic. No functional changes detected. Ready to merge.This replacement also applies to the corresponding loops at:
- Lines 1095–1114
- Lines 1121–1139
- Lines 1154–1172
- Lines 1179–1199
- Lines 1208–1216
- Lines 1311–1330
- Lines 1337–1355
- Lines 1369–1387
- Lines 1394–1415
- Lines 1424–1443
zip/zip_test.go (2)
29-37: Range-over-integers conversions look correct and improve clarity
- for i := range nFiles iterates expected [0..nFiles-1] in both writer and reader checks.
- The inner generators (range numRec / range chunks) are semantically identical to old counted loops.
These are good modernizations with no observable behavior changes in the tests.
Also applies to: 49-54, 354-365, 567-573, 599-605
426-428: suffixSaver.Write: min() bound is correct and simpleradd := min(len(p), space) is equivalent to the previous manual clamp and improves readability. Looks good.
zstd/dict.go (1)
451-456: Range-over-integers: OK in 1.22+ and reads wellThe retries loop and bailout literal seeding use for range N; identical semantics to the previous counted loops. Looks good.
Also applies to: 469-471
zstd/decoder_test.go (1)
228-234: Modernized range loops in tests: finefor i := range 256 and for range 3 are idiomatic with Go ≥ 1.22 and equivalent to prior counted loops. No functional changes.
Also applies to: 293-299, 358-364, 874-892, 895-909
zstd/enc_dfast.go (5)
152-157: Good consolidation of the backward-extend bound (tMin).Replacing repeated inline expressions with a single tMin := max(s-e.maxMatchOff, 0) improves readability and prevents accidental sign/overflow mistakes. Same applies to other tMin insertions below.
570-575: NoHist backward extension without maxMatchLength guard looks intentional.EncodeNoHist explicitly states “we do not need to check for max match length,” so dropping the l < maxMatchLength condition is consistent. No issues spotted.
949-967: Precomputing long/short hashes and marking shards dirty is correct and clearer.
- Computes longHash1/2, updates longTable once each, and marks both long shards dirty.
- Shifts cv0/cv1 by 8, increments offsets, and then computes short-hash indices and marks both short shards dirty.
This aligns with the dict-path cache invalidation strategy and avoids duplicate recomputation. Good change.
1083-1090: Bulk copy from dictLongTable via array conversion is sound.The slice-to-array conversion copies the full table efficiently. Preconditions are satisfied earlier (matching lengths and lastDictID), and shard-dirty flags are reset. LGTM.
1096-1099: Shard-level copy optimization is correct.Converting the shard slices to [dLongTableShardSize]tableEntry avoids per-element loops and is safe given the constant bound. Flags are cleared per shard after copy. LGTM.
flate/deflate_test.go (4)
241-286: Range-over-integers in testSync is appropriate.for i := range 2 preserves the original two-iteration semantics and keeps i for branching. Compiles with the minimum supported Go per this PR. LGTM.
343-347: Range-over-integers in testToFromWithLimit is appropriate.for i := range 10 retains index use for level selection. LGTM.
473-479: Use of ‘for range 131072’ is concise and keeps semantics.Index is unused, so the blank-range form is ideal here. LGTM.
495-497: Loop modernizations to ‘for range 1024’ are correct.Index is unused; the new syntax removes noise without changing behavior. LGTM.
Also applies to: 560-562, 568-570
flate/huffman_bit_writer.go (3)
306-313: Use min() to clamp 16-run length is equivalent and clearer.n := min(6, count) keeps 16’s semantics (3–6 repeats) with simpler code. LGTM.
317-323: Use min() to clamp 18-run length is equivalent and clearer.n := min(138, count) matches the 11–138 range for code 18 with a simpler clamp. LGTM.
469-472: Range-over-integers for codegen header is fine.for i := range numCodegens iterates 0..numCodegens-1 and indexes codegenOrder like before. No semantic change. LGTM.
flate/deflate.go (6)
237-241: Loop rewrite in fillWindow is correct and tidier.for j := range loops with end := min(...) keeps bounds correct and removes the explicit branch. LGTM.
269-275: findMatch: bound clamps via min() improve clarity.minMatchLook and nice are correctly bounded; no functional change. LGTM.
283-283: findMatch: minIndex via max() is clearer.max(pos-windowSize, 0) is equivalent to previous clamp. LGTM.
472-472: deflateLazy: minIndex via max() is equivalent. LGTM.
491-493: deflateLazy: end bound uses min(lookahead, maxMatchLength+checkOff).This keeps checks within available lookahead while preventing overscan; consistent with following uses. LGTM.
589-591: Hash recomputation window clamps are equivalent and clearer.end := min(newIndex, s.maxInsertIndex) and startindex := min(s.index+1, s.maxInsertIndex) match earlier behavior. LGTM.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
zstd/_generate/go.mod (1)
5-5: Pinned Go toolchain diverges from CI: audit & unifyOur audit shows the following
toolchaindirectives across submodules:
zstd/_generate/go.mod: toolchain go1.24.2s2/_generate/go.mod: toolchain go1.22.4s2/cmd/_s2sx/go.mod: toolchain go1.24.2huff0/_generate/go.mod: toolchain go1.24.2Meanwhile, CI workflows (
.github/workflows/go.ymlandrelease.yml) install and test with Go 1.25.x (and a matrix including 1.23.x, 1.24.x, 1.25.x) . This misalignment means localgo generateruns could use an older compiler than CI, potentially yielding different codegen outputs or masking compatibility issues.Consider the following options:
- Bump all
toolchaindirectives togo1.25.xto match CI.- Drop the
toolchainlines altogether, relying on the runner’s default Go (1.25.x).- Keep specific pins (e.g. 1.24.2) only if codegen reproducibility truly depends on that exact version—in which case, also update CI to include that version in its matrix.
Please confirm which behavior you intend and update accordingly.
s2/cmd/_s2sx/go.mod (1)
5-5: Align or Remove Patch-Level Toolchain Pin in s2/cmd/_s2sx/go.modOur audit confirms that this submodule’s
toolchain go1.24.2is out of sync with other parts of the repo and the CI matrix:• Root go.mod has no
toolchaindirective (onlygo 1.23).
• Generated modules:
– s2/_generate/go.mod →toolchain go1.22.4
– zstd/_generate/go.mod →toolchain go1.24.2
– huff0/_generate/go.mod →toolchain go1.24.2
– s2/cmd/_s2sx/go.mod →toolchain go1.24.2
• GitHub Actions workflows build against Go versions[1.23.x, 1.24.x, 1.25.x](and release jobs pin1.25.x).Pinning a patch-level version here means if the project-wide standard advances to, say, Go 1.25.x, this submodule will still require 1.24.2, creating potential drift. You have three paths:
- Remove the
toolchain go1.24.2line entirely and let CI/workflows dictate the compiler.- Unify all module-level pins to the same patch-level version if there’s a non-negotiable fix in 1.24.2 (e.g., known compiler bug).
- Bump (or future-proof) to the next major patch (e.g.,
toolchain go1.25.0) if you expect the code here requires features/fixes in Go 1.25.If you opt to remove the directive:
--- s2/cmd/_s2sx/go.mod @@ -3,3 +3,0 @@ -toolchain go1.24.2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
huff0/_generate/go.mod(1 hunks)s2/cmd/_s2sx/go.mod(1 hunks)zstd/_generate/go.mod(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- huff0/_generate/go.mod
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: build (1.23.x, macos-latest)
- GitHub Check: build (1.25.x, windows-latest)
- GitHub Check: build (1.24.x, macos-latest)
- GitHub Check: build (1.24.x, windows-latest)
- GitHub Check: build (1.25.x, ubuntu-latest)
- GitHub Check: build (1.24.x, ubuntu-latest)
- GitHub Check: build (1.23.x, windows-latest)
- GitHub Check: build (1.23.x, ubuntu-latest)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
s2/cmd/_s2sx/go.mod (1)
3-3: Go version bumped to 1.23 — LGTMThis aligns with the PR objective to deprecate Go 1.22.
Optionally re-run the cross-repo check to ensure no
go 1.22directives remain:#!/bin/bash set -euo pipefail echo "All go directives:" rg -n --hidden -g '!**/vendor/**' -g go.mod -e '^go\s+1\.[0-9]+\b' || true echo echo "Checking for go 1.22:" if rg -n --hidden -g '!**/vendor/**' -g go.mod -e '^go\s+1\.22(\.0)?\b'; then echo "Found 1.22 above — please bump those to 1.23" exit 1 fi
🧹 Nitpick comments (2)
s2/cmd/_s2sx/go.mod (2)
7-9: Future-proof the local replace by removing the version constraintThe current replace only applies when
v1.11.9is required. If therequireis bumped later, the replace will silently stop applying. Prefer a version-agnostic replace to ensure any version of the root module resolves to the local checkout during development.Apply:
-replace github.com/klauspost/compress v1.11.9 => ../../.. +replace github.com/klauspost/compress => ../../..This keeps
require github.com/klauspost/compress v1.11.9intact while making the replace resilient to future version bumps.
5-5: Align or Remove Submodule Go Toolchain PinningThe
toolchain go1.24.2directive appears consistently in three submodulego.modfiles but is not present in the root module. To avoid unexpected automatic downloads or version skew across your workspace, consider either removing these pins or elevating the same directive to your rootgo.mod.• Instances of the directive:
zstd/_generate/go.mod(line 5)huff0/_generate/go.mod(line 5)s2/cmd/_s2sx/go.mod(line 5)If you opt to drop the submodule pins, apply the following patch to each file:
-toolchain go1.24.2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
huff0/_generate/go.mod(2 hunks)s2/_generate/go.mod(2 hunks)s2/cmd/_s2sx/go.mod(1 hunks)snappy/xerial/xerial_test.go(2 hunks)zstd/_generate/go.mod(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- huff0/_generate/go.mod
- s2/_generate/go.mod
- snappy/xerial/xerial_test.go
- zstd/_generate/go.mod
🔇 Additional comments (1)
s2/cmd/_s2sx/go.mod (1)
1-1: No lingering references to the old module path foundI ran the ripgrep scan you provided against the entire repo (excluding
vendor/), searching for any occurrence ofs2/cmd/s2sxand got zero matches. That indicates there are no internal references to the old module path left in this codebase.• Repository search command:
rg -n -C2 --hidden -g '!**/vendor/**' -e 's2/cmd/s2sx\b'→ No matches found.
You can now safely consider the module path rename complete for this repository. If you have external documentation, CI scripts, or downstream modules that reference the old path, please update those separately.
This PR contains the following updates: | Package | Change | Age | Confidence | |---|---|---|---| | [github.com/klauspost/compress](https://github.com/klauspost/compress) | `v1.18.0` -> `v1.18.1` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>klauspost/compress (github.com/klauspost/compress)</summary> ### [`v1.18.1`](https://github.com/klauspost/compress/releases/tag/v1.18.1) [Compare Source](klauspost/compress@v1.18.0...v1.18.1) #### What's Changed - zstd: Fix incorrect buffer size in dictionary encodes by [@​klauspost](https://github.com/klauspost) in [#​1059](klauspost/compress#1059) - s2: check for cap, not len of buffer in EncodeBetter/Best by [@​vdarulis](https://github.com/vdarulis) in [#​1080](klauspost/compress#1080) - zstd: Add simple zstd EncodeTo/DecodeTo functions by [@​klauspost](https://github.com/klauspost) in [#​1079](klauspost/compress#1079) - zlib: Avoiding extra allocation in zlib.reader.Reset by [@​travelpolicy](https://github.com/travelpolicy) in [#​1086](klauspost/compress#1086) - gzhttp: remove redundant err check in zstdReader by [@​ryanfowler](https://github.com/ryanfowler) in [#​1090](klauspost/compress#1090) - Run modernize. Deprecate Go 1.22 by [@​klauspost](https://github.com/klauspost) in [#​1095](klauspost/compress#1095) - flate: Simplify matchlen by [@​klauspost](https://github.com/klauspost) in [#​1101](klauspost/compress#1101) - flate: Add examples by [@​klauspost](https://github.com/klauspost) in [#​1102](klauspost/compress#1102) - flate: Use exact sizes for huffman tables by [@​klauspost](https://github.com/klauspost) in [#​1103](klauspost/compress#1103) - flate: Faster load+store by [@​klauspost](https://github.com/klauspost) in [#​1104](klauspost/compress#1104) - Add notice to S2 about MinLZ by [@​klauspost](https://github.com/klauspost) in [#​1065](klauspost/compress#1065) #### New Contributors - [@​wooffie](https://github.com/wooffie) made their first contribution in [#​1069](klauspost/compress#1069) - [@​vdarulis](https://github.com/vdarulis) made their first contribution in [#​1080](klauspost/compress#1080) - [@​travelpolicy](https://github.com/travelpolicy) made their first contribution in [#​1086](klauspost/compress#1086) - [@​ryanfowler](https://github.com/ryanfowler) made their first contribution in [#​1090](klauspost/compress#1090) **Full Changelog**: <klauspost/compress@v1.18.0...v1.18.1> </details> --- ### Configuration 📅 **Schedule**: Branch creation - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC), Automerge - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xNTIuOSIsInVwZGF0ZWRJblZlciI6IjQxLjE1Mi45IiwidGFyZ2V0QnJhbmNoIjoiZm9yZ2VqbyIsImxhYmVscyI6WyJkZXBlbmRlbmN5LXVwZ3JhZGUiLCJ0ZXN0L25vdC1uZWVkZWQiXX0=--> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9786 Reviewed-by: Earl Warren <[email protected]> Co-authored-by: Renovate Bot <[email protected]> Co-committed-by: Renovate Bot <[email protected]>
Running: https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/modernize
Summary by CodeRabbit
Chores
Refactor
Tests
Bug Fixes