Mike Percy has posted comments on this change. Change subject: Rolling-upgrades compat for changing TSRegistrationPB ......................................................................
Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/4062/5/src/kudu/master/ts_descriptor.cc File src/kudu/master/ts_descriptor.cc: PS5, Line 60: static bool HostPortPBsEqual(const ::google::protobuf::RepeatedPtrField<::kudu::HostPortPB>& pb1, : const ::google::protobuf::RepeatedPtrField<::kudu::HostPortPB>& pb2) > Not related to your change, but can you get away with HostPortPB instead of Done http://gerrit.cloudera.org:8080/#/c/4062/5/src/kudu/util/net/net_util.h File src/kudu/util/net/net_util.h: PS5, Line 55: bool Equals(const HostPort& other) const { : return port_ == other.port_ && host_ == other.host_; : } > Since this is new, consider dropping it in favor of just operator==, which Done Line 59: bool operator==(const HostPort& other) const { > Google style guide says const binary operators should be defined as free fu Hmm, didn't know about that one. ODR requires me to rejigger a couple things. Done. -- To view, visit http://gerrit.cloudera.org:8080/4062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
