> On Sept. 12, 2014, 11:39 p.m., Ben Mahler wrote: > > Thanks for following up! > > > > Not your fault, but the current design of enable() seems a bit unfortunate, > > because we will print things excessively unless we use static variables as > > you've done here. > > What about the following instead? > > > > (1) Add a method called 'disabled()' that returns a list of disabled test > > names. > > (2) Inside 'disabled()', we can print the disablement messages once at the > > beginning, then loop over the tests to construct the list of disabled test > > names to return. > > (3) Inside 'Environment::Environment()', we use disabled() to construct the > > full 'disabled' string. > > > > Does that seem cleaner? > > > > The static variables seem good for now, modulo my comment below. Consider > > adding a TODO to clean this up if you go this route!
Ok I've done a refactor based on your comments, let me know what you think. - Timothy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25569/#review53246 ----------------------------------------------------------- On Sept. 16, 2014, 7:07 a.m., Timothy Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25569/ > ----------------------------------------------------------- > > (Updated Sept. 16, 2014, 7:07 a.m.) > > > Review request for mesos and Ben Mahler. > > > Repository: mesos-git > > > Description > ------- > > Review: https://reviews.apache.org/r/25569 > > Refactor how validation is done in the environment by defining a standard > test filter interface, and only execute the validation once. > > > Diffs > ----- > > src/tests/environment.cpp 2274251aaf653d83c2d03ef2186763978067a747 > > Diff: https://reviews.apache.org/r/25569/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Timothy Chen > >
