----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23997/#review48926 -----------------------------------------------------------
Ship it! Looks good, just a few minor cleanups and we can get this committed! src/master/main.cpp <https://reviews.apache.org/r/23997/#comment85719> Please use the existing TODO format: add your handle and make it a sentence (end with a period). src/master/main.cpp <https://reviews.apache.org/r/23997/#comment85717> Looks like we should only parse the zk.get() string if it doesn't start with "file://": Try<URL> url; if (strings::startsWith(zk.get(), "file://")) { ... } else { url = URL::parse(zk.get()); } src/master/main.cpp <https://reviews.apache.org/r/23997/#comment85718> I see this is copied, but this should include the error reason to help the operator: EXIT(1) << "Failed to read from file at '" + path + "': " << read.error(); - Ben Mahler On July 28, 2014, 10:36 p.m., Ken Sipe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23997/ > ----------------------------------------------------------- > > (Updated July 28, 2014, 10:36 p.m.) > > > Review request for mesos, Adam B and Ben Mahler. > > > Bugs: MESOS-1635 > https://issues.apache.org/jira/browse/MESOS-1635 > > > Repository: mesos-git > > > Description > ------- > > Fixes MESOS-1635 "zk flag fails when specifying a file and the replicated > logs" > > replaced the current TODO with code :) The code is consistent with similar > types of checks elsewhere in code. > > > Diffs > ----- > > src/master/main.cpp 4d39c355eae948ede492d84de2e58a26756b781a > > Diff: https://reviews.apache.org/r/23997/diff/ > > > Testing > ------- > > The code change is simple and no unit test is provided. It compiles, passes > style check and is functional via manual check. > > > Thanks, > > Ken Sipe > >
