Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23651 )
Change subject: KUDU-1457 [5/n] Enable IPv6-only mode for rpc ...................................................................... Patch Set 3: Code-Review+2 (4 comments) Overall LGTM, just a couple of nits to address. Thanks for revving this patch! http://gerrit.cloudera.org:8080/#/c/23651/2/src/kudu/rpc/rpc-test.cc File src/kudu/rpc/rpc-test.cc: http://gerrit.cloudera.org:8080/#/c/23651/2/src/kudu/rpc/rpc-test.cc@2247 PS2, Line 2247: case I > ASSERT_OK returns status from ctor. I have removed the ctor and replaced wi ASSERT_OK doesn't return Status, but a void expression. But compilation would fail with ASSERT_OK() here, indeed. Moving all the preparation steps into the SetUp() method is the best option, thanks. http://gerrit.cloudera.org:8080/#/c/23651/2/src/kudu/rpc/rpc-test.cc@2329 PS2, Line 2329: } : > The error string : Yes, this looks good to me. Thanks for the update. http://gerrit.cloudera.org:8080/#/c/23651/3/src/kudu/rpc/rpc-test.cc File src/kudu/rpc/rpc-test.cc: http://gerrit.cloudera.org:8080/#/c/23651/3/src/kudu/rpc/rpc-test.cc@2294 PS3, Line 2294: Status s; : s = s4.Connect(target_addr); nit: could this be just Status s = s4.Connect(target_addr); ? http://gerrit.cloudera.org:8080/#/c/23651/3/src/kudu/rpc/rpc-test.cc@2319 PS3, Line 2319: Status s; : s = s6.Connect(target_addr); nit: please shorten to Status s = s6.Connect(target_addr); ? -- To view, visit http://gerrit.cloudera.org:8080/23651 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic84ba301d0faa36e928eec54872d02f28a4f793f Gerrit-Change-Number: 23651 Gerrit-PatchSet: 3 Gerrit-Owner: Ashwani Raina <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Wed, 31 Dec 2025 18:05:49 +0000 Gerrit-HasComments: Yes
