Alexey Serbin has posted comments on this change. Change subject: rpc: move UserCredentials to separate .h/cc file ......................................................................
Patch Set 1: Code-Review+2 (6 comments) http://gerrit.cloudera.org:8080/#/c/5997/1/src/kudu/rpc/user_credentials.cc File src/kudu/rpc/user_credentials.cc: PS1, Line 46: // Does not print the password. nit: is it relevant? PS1, Line 51: size_t seed = 0; : if (has_real_user()) { : boost::hash_combine(seed, real_user()); : } : return seed; nit: would boost::hash_value(real_user_) suffice? PS1, Line 59: real_user() == other.real_user(); nit: is it possible to get rid of function call (even if inlined) here? http://gerrit.cloudera.org:8080/#/c/5997/1/src/kudu/rpc/user_credentials.h File src/kudu/rpc/user_credentials.h: PS1, Line 26: & password nit: is it relevant? PS1, Line 29: // TODO(mpercy): this is actually used server side too -- should : // we instead introduce a RemoteUser class or something? nit: is this TODO still relevant? PS1, Line 36: bool has_real_user() const; : void set_real_user(const std::string& real_user); : const std::string& real_user() const { return real_user_; } nit: why 'real'? Is there any other? -- To view, visit http://gerrit.cloudera.org:8080/5997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I679d248e83da0aeed47957d958b0152ca4bac6cd Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
