----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35711/#review114435 -----------------------------------------------------------
Ship it! Looks great! Just a few minor items that I can fix myself. I'll commit this shortly. src/common/roles.cpp (line 58) <https://reviews.apache.org/r/35711/#comment175280> s/out this/this out/ s/satisfies/satisfy/ src/master/master.cpp (lines 643 - 644) <https://reviews.apache.org/r/35711/#comment175281> `LOG(WARNING) << "Duplicate values in '--roles': " << flags.roles.get();` include/mesos/roles.hpp (line 36) <https://reviews.apache.org/r/35711/#comment175284> Need double blank lines between these, since they're at the top level (not within a class). src/common/roles.cpp (line 30) <https://reviews.apache.org/r/35711/#comment175285> s/to support/supporting/ src/common/roles.cpp (line 68) <https://reviews.apache.org/r/35711/#comment175287> We can pull this (and its comment) out into a `static const string INVALID_CHARACTERS` in the namespace. - Adam B On Jan. 13, 2016, 10 p.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35711/ > ----------------------------------------------------------- > > (Updated Jan. 13, 2016, 10 p.m.) > > > Review request for mesos, Adam B, Jie Yu, and Michael Park. > > > Bugs: MESOS-2210 > https://issues.apache.org/jira/browse/MESOS-2210 > > > Repository: mesos > > > Description > ------- > > Disallow special characters in role name. > > > Diffs > ----- > > docs/roles.md af1adad7ec8122fd10f7de44848014b850416bcd > include/mesos/roles.hpp PRE-CREATION > src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 > src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 > src/common/roles.cpp PRE-CREATION > src/master/master.cpp c122c30d943813fc3ce9e7025783c7231809b022 > src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 > src/tests/role_tests.cpp 373ae267b85588fe491ab0a0ce8aa195f971aac3 > > Diff: https://reviews.apache.org/r/35711/diff/ > > > Testing > ------- > > make -j8 check > > > Thanks, > > haosdent huang > >
