Asthenia0412 commented on PR #1376: URL: https://github.com/apache/arrow-go/pull/1376#issuecomment-6258568582
@zeroshade All three inline comments addressed in the latest commits: 1. **Doc comment** — The comment above WithZstdLevel now documents the actual 4-bucket EncoderLevelFromZstd mapping (<3 / 3-5 / 6-9 / >=10) and explicitly notes that callers can't expect finer granularity than four discrete speed presets. 2. **Implicit ZSTD codec** — WithZstdLevel(level) now sets cfg.codec = flatbuf.CompressionTypeZSTD in addition to storing the level, so callers can write just WithZstdLevel(5) without also needing WithZstd(). 3. **CompressionSettings bundling** — The three adjacent compression arguments (codec, zstdLevel, compressNP) are now grouped into a compressionSettings struct. newRecordEncoder dropped from 9 to 7 positional args, and all three call sites (Writer.getRecordEncoder, FileWriter.getRecordEncoder, GetRecordBatchPayload) plus the test/bench call sites were updated. I also expanded the TestWithZstdLevel table in the earlier push to cover NewWriter / NewFileWriter round-trips, same-preset equivalence (levels 1 and 2 both map to SpeedFastest and produce byte-identical output), and WithZstd() vs WithZstdLevel(0) equivalence. go test ./arrow/ipc/... is green. PTAL! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
