Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23707 )

Change subject: KUDU-1457 [8/n] Parameterize tests with ip config
......................................................................


Patch Set 5: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/23707/4/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:

http://gerrit.cloudera.org:8080/#/c/23707/4/src/kudu/util/net/net_util.h@285
PS4, Line 285: ip_config_mode
> Are you pointing to just changing the comment to add reference to ParseIPMo
The idea was to limit the scope of references, removing the mention of 
ip_config_mode flag.  It's just a tiny nit, so feel free to ignore this.

Should be OK as is.


http://gerrit.cloudera.org:8080/#/c/23707/4/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:

http://gerrit.cloudera.org:8080/#/c/23707/4/src/kudu/util/test_util.cc@762
PS4, Line 762: void PrintTo(IPMode mode, std::ostream* os) {
             :   *os << IPModeToString(mode);
             : }
> My intention is to restrict this the usage to gtest failure messages and no
I'm not sure I understand the concern about 'spreading elsewhere'.  Both 
versions are declared in test_util.h, so both are spread where test_util.h is 
included.  And test_util.h in supposed to be included into test-only .cc files.

What wasn't clean with defining the operator for outputting IPMode to 
std::ostream?



--
To view, visit http://gerrit.cloudera.org:8080/23707
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic72efea2045b12300ccf0e80873c405828c15a30
Gerrit-Change-Number: 23707
Gerrit-PatchSet: 5
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: Mon, 02 Feb 2026 18:27:05 +0000
Gerrit-HasComments: Yes

Reply via email to