> On March 27, 2017, 7:01 p.m., Kevin Klues wrote: > > src/slave/flags.cpp > > Lines 152-158 (original), 152-169 (patched) > > <https://reviews.apache.org/r/55065/diff/1/?file=1592795#file1592795line152> > > > > We typically don't create duplicate entries like this when simply > > customizing the default value. Instead we use a lambda with the #ifdef's > > inside of it. For example, > > > > https://github.com/apache/mesos/blob/master/src/slave/flags.cpp#L212 > > Till Toenshoff wrote: > I am not sure we really need to come up with a lambda for such static > default - see > https://github.com/apache/mesos/blob/42f43ceb3b4b375357281030fb3816e548dac763/src/slave/flags.cpp#L670 > Let's not make this more complicated than needed. > > Kevin Klues wrote: > Sure that makes sense. No need for a lambda in this case, but the #ifdef > should only be around the new default value parameter, not a duplicate of the > entire flags field.
On a second thought, as user can anyways configure to local path if required, as mentioned in flag description. I would like to drop these changes. - Ayanampudi ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55065/#review170209 ----------------------------------------------------------- On March 27, 2017, 9:25 a.m., Ayanampudi Varsha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55065/ > ----------------------------------------------------------- > > (Updated March 27, 2017, 9:25 a.m.) > > > Review request for mesos, Till Toenshoff and Vinod Kone. > > > Bugs: MESOS-6742 > https://issues.apache.org/jira/browse/MESOS-6742 > > > Repository: mesos > > > Description > ------- > > Added support for s390x architecture > There are 2 issues addressed in this patch: > 1. LdcacheTest.Parse test case fails on s390x machines. > 2. From the value of flag docker_registry in slave/flags.cpp, amd64 images > get downloaded due to which test cases fail on s390x with "Exec format Error" > > > Diffs > ----- > > src/linux/ldcache.cpp 6260496672cd0226a24ea1b34003a48af9678a63 > src/slave/flags.cpp 1eccea920338032173be4df6c374ec50dbd2eaf3 > > > Diff: https://reviews.apache.org/r/55065/diff/1/ > > > Testing > ------- > > make check > make check GTEST_FILTER="LdcacheTest.Parse" > > > Thanks, > > Ayanampudi Varsha > >
