Jens-G commented on PR #3646: URL: https://github.com/apache/thrift/pull/3646#issuecomment-5028070891
### Code review Found 1 issue: 1. Unrelated behavior changes bundled into a scope-limited bug fix (AGENTS.md ยง7: "When fixing a bug or addressing an issue, scope the change to ONLY that issue... Ship bug fixes as standalone pull requests by default... do not mix fixes into existing/unrelated work"). THRIFT-6105's title/body/ticket describe only read-position preservation, but the diff also replaces `Check_Type(buffer_value, T_STRING)` with `StringValue(buffer_value)` (silently widening accepted input from "must be a String" to "anything coercible via `to_str`"), and adds a new pre-loop EOF/empty-destination check that changes exception precedence (`EOFError` now wins over `FrozenError` when zero bytes are copied) โ both squarely in THRIFT-6100's stated territory, not THRIFT-6105's. https://github.com/apache/thrift/blob/2fa08c0d36afc15fbab00df457e6bd9fe3c54824/lib/rb/ext/memory_buffer.c#L134-L147 ๐ค Generated with [Claude Code](https://claude.ai/code) <sub>- If this code review was useful, please react with ๐. Otherwise, react with ๐.</sub> -- 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]
