> On June 10, 2014, 8:37 a.m., Adam B wrote: > > src/master/master.cpp, lines 1097-1100 > > <https://reviews.apache.org/r/22284/diff/2/?file=606172#file606172line1097> > > > > These Option<Error>'s read weird to me. I would expect > > future.get().isSome() to be a positive case, not an error case. > > Using Try<Nothing>'s instead gives "if(future.get().isError()) { > > //handle error }", which reads a lot better to me. > > Vinod Kone wrote: > See the discussion in the "authorize tasks" ticket regarding this. Since > the type is Option<Error>, a '.isSome()' tells one that it is an error. We > have used this format before (see status update manager, group). > > Ben Mahler wrote: > How about calling it 'validationError' so that 'validationError.isSome()' > reads better? > > Adam B wrote: > +1 I like this. "future" is so ambiguous.
done. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22284/#review45202 ----------------------------------------------------------- On June 10, 2014, 7:18 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22284/ > ----------------------------------------------------------- > > (Updated June 10, 2014, 7:18 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Bugs: MESOS-1307 > https://issues.apache.org/jira/browse/MESOS-1307 > > > Repository: mesos-git > > > Description > ------- > > Added authorization for roles during framework (re-)registration. > > > Diffs > ----- > > src/master/flags.hpp 486335970ef05b345c5584ac012dde63437ef149 > src/master/master.hpp 26af1139a43a62b91712acd158b24a8977c81d3f > src/master/master.cpp c18ccc4a1770cd68e4c3cb4b5f8ab912515ab613 > src/tests/master_authorization_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/22284/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
