----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20423/#review63577 -----------------------------------------------------------
I think I agree with Vinod that this is just movement without adding any value, for now, given that these are not independent of the Master. In fact, to understand these, one needs to understand the internals of the Master, which now requires even more non-local reasoning (need to be jumping across files to understand these). IMO this makes it harder to understand, and I would punt on this change in favor of first trying to clean these up to make them unit-testable. Until then, the loss of local reasoning in this change seems really unfortunate. This is not yours, but the existing names could be improved. What is a "checker"? And yes, these are "Visitors", but they are all ultimately for validation. So, how about we call these Validators? TaskIDValidator is a TaskValidator, UniqueOfferIDValidator is an OfferValidator. - Ben Mahler On Dec. 2, 2014, 8:36 p.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20423/ > ----------------------------------------------------------- > > (Updated Dec. 2, 2014, 8:36 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Bugs: MESOS-1064 > https://issues.apache.org/jira/browse/MESOS-1064 > > > Repository: mesos-git > > > Description > ------- > > This is the first step toward being able to write independent unit tests for > the validation visitors. > > > Diffs > ----- > > src/Makefile.am 86161fe7a8bdd86958d24adb74d434cd92d7dfb8 > src/master/master.cpp 8fcda4b9b5857f14cff8f6af2de31cca0208b88d > src/master/offer_checker.hpp PRE-CREATION > src/master/offer_checker.cpp PRE-CREATION > src/master/task_checker.hpp PRE-CREATION > src/master/task_checker.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/20423/diff/ > > > Testing > ------- > > make check. > ran Java test framework. > > > Thanks, > > Dominic Hamon > >
