----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65588/#review197394 -----------------------------------------------------------
Thanks for cleaning this up! Looks great, only minor thing that I've noticed and which isn't in the scope of this diff is some inconsistent naming in `slave.cpp`. E.g., there's `operationUUID` vs. `operationUuid`, especially in `Slave::handleResourceProviderMessage`. It would be great I we could agree on a convention here. src/common/protobuf_utils.cpp Line 467 (original), 467 (patched) <https://reviews.apache.org/r/65588/#comment277521> How about we create a helper function `mesos::UUID createRandomUUID()` in `protobuf_utils` even if that function would be almost trivial? This would allow us to have the expectations of a UUID (i.e. it's `UUID::random().toBytes()`, not `UUID::random().toString()`) in a single place. Here and below, where `->set_value(id::UUID::random().toBytes())` is used. - Jan Schlicht On Feb. 9, 2018, 6:27 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65588/ > ----------------------------------------------------------- > > (Updated Feb. 9, 2018, 6:27 p.m.) > > > Review request for mesos, Jie Yu and Jan Schlicht. > > > Bugs: MESOS-8382 > https://issues.apache.org/jira/browse/MESOS-8382 > > > Repository: mesos > > > Description > ------- > > Used proto UUID instead stout UUID internally for operation IDs. > > > Diffs > ----- > > include/mesos/resource_provider/resource_provider.proto > e96a40493c351a7242465c8591cae981abc92f24 > src/common/protobuf_utils.hpp 9a940db188436b9c5a39b0637cb4c15ee2ab5266 > src/common/protobuf_utils.cpp b5c2997ada8362e42150fa3cfd762120e5ea715f > src/master/master.hpp b434d2398b8815811345b6586ca586d2025cb2a2 > src/master/master.cpp d7d22866f7a4eb87bd8949efafc97e828e7d4b94 > src/resource_provider/manager.cpp cc195a3d35b93dd6493951de1ff8a1cb8a886377 > src/resource_provider/message.hpp 2511af611edc94e3e3a78fce475bc8cd85ffc3f9 > src/slave/slave.hpp 30151c4886e12e9183a971b86b854e28a8ca1b39 > src/slave/slave.cpp f98f37321872d090176b7cc50873fc3c627773f5 > > > Diff: https://reviews.apache.org/r/65588/diff/1/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
