Adar Dembo has posted comments on this change. Change subject: rw_mutex: add configurable priority ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/3603/1/src/kudu/util/rw_mutex.h File src/kudu/util/rw_mutex.h: Line 31: enum Priority { > consider making this an enum class. Neat, wasn't aware of this feature. Unfortunately, it makes logging the enum value in the case where it somehow isn't one these two values difficult: /home/adar/Source/kudu/src/kudu/util/rw_mutex.cc: In member function ‘void kudu::RWMutex::Init(kudu::RWMutex::Priority)’: /home/adar/Source/kudu/src/kudu/util/rw_mutex.cc:53:41: error: no match for ‘operator<<’ (operand types are ‘std::basic_ostream<char>’ and ‘kudu::RWMutex::Priority’) LOG(FATAL) << "Unknown priority: " << prio; The solution proposed in http://stackoverflow.com/a/11421471 is pretty ugly, so unless you know of a better way, I'm going to keep the code as-is. Line 57: void Init(Priority prio); > any reason to have a separate init instead of doing it in the ctor as befor My understanding is that there's no way to "chain" constructors in C++, so I pulled the common initialization code into Init() and am invoking it from both constructors. -- To view, visit http://gerrit.cloudera.org:8080/3603 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16ba6cd041f126c94e63fa07a1e84c88db6778d7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Dan Burkert <d...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
