RockteMQ-AI commented on PR #10590: URL: https://github.com/apache/rocketmq/pull/10590#issuecomment-5015733553
### What changed - Added `-t offset` option to decode offset-format message IDs into `StoreHost` + `CommitLogOffset` - `MessageDecoder.decodeMessageId()` is used for full parsing — this correctly addresses the review suggestion - Tests cover unique/offset/IPv4/IPv6 scenarios and error handling ### Assessment The new commit properly resolves the review feedback. The offsetId parsing logic is clean and well-tested. **Minor observations:** 1. **Command name case change**: `DecodeMessageId` → `decodeMessageId` — this is a breaking change for scripts using `mqadmin DecodeMessageId`. If backward compatibility matters, consider keeping the original casing or documenting the rename in release notes. 2. **`-i` option now required**: Previously optional (with interactive prompt), now marked `requiredOpt`. This is a behavior change — users who relied on the interactive prompt will now get an error. Consider whether this is intentional. 3. **`formatStoreHost` duplication**: The same hex-to-InetSocketAddress logic exists in both `DecodeMessageIdCommand` and `DecodeMessageIdCommandTest`. Minor — could extract to a shared utility if desired. Overall the implementation looks solid. 👍 -- 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]
