> On Feb. 24, 2014, 6:36 p.m., Kevin Sweeney wrote: > > src/main/python/apache/aurora/admin/mesos_maintenance.py, line 73 > > <https://reviews.apache.org/r/18334/diff/3/?file=502368#file502368line73> > > > > Inclined to revert this - better to explicitly call out a dependency on > > system time IMO.
done > On Feb. 24, 2014, 6:36 p.m., Kevin Sweeney wrote: > > src/main/python/apache/aurora/client/commands/maintenance.py, lines 53-56 > > <https://reviews.apache.org/r/18334/diff/3/?file=502372#file502372line53> > > > > DRY - these options are used multiple times. Consider factoring them as > > constants. fixed > On Feb. 24, 2014, 6:36 p.m., Kevin Sweeney wrote: > > src/test/python/apache/aurora/client/commands/test_maintenance.py, line 43 > > <https://reviews.apache.org/r/18334/diff/3/?file=502376#file502376line43> > > > > make_mock_options would be more descriptive. also docstring is not > > informative, consider dropping it. fixed > On Feb. 24, 2014, 6:36 p.m., Kevin Sweeney wrote: > > src/test/python/apache/aurora/client/commands/test_maintenance.py, line 88 > > <https://reviews.apache.org/r/18334/diff/3/?file=502376#file502376line88> > > > > parens aren't necessary fixed > On Feb. 24, 2014, 6:36 p.m., Kevin Sweeney wrote: > > src/test/python/apache/aurora/client/commands/test_maintenance.py, line 103 > > <https://reviews.apache.org/r/18334/diff/3/?file=502376#file502376line103> > > > > parens not necessary fixed > On Feb. 24, 2014, 6:36 p.m., Kevin Sweeney wrote: > > src/test/python/apache/aurora/client/commands/test_maintenance.py, line 129 > > <https://reviews.apache.org/r/18334/diff/3/?file=502376#file502376line129> > > > > no parens needed. fixed > On Feb. 24, 2014, 6:36 p.m., Kevin Sweeney wrote: > > src/test/python/apache/aurora/client/commands/test_maintenance.py, line 136 > > <https://reviews.apache.org/r/18334/diff/3/?file=502376#file502376line136> > > > > Not sure how I feel about this style of mock - I'd prefer the class > > under test to explicitly call out "I'm using system time" to avoid missed > > mock coverage causing the test suite to slow down. hm.. any chance you have suggestions on how to pass this through? I'd prefer not to add it to the command line itself, I think - Joe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18334/#review35362 ----------------------------------------------------------- On Feb. 24, 2014, 10:19 a.m., Joe Smith wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18334/ > ----------------------------------------------------------- > > (Updated Feb. 24, 2014, 10:19 a.m.) > > > Review request for Aurora, Kevin Sweeney, Mark Chu-Carroll, and Brian Wickman. > > > Bugs: AURORA-223 > https://issues.apache.org/jira/browse/AURORA-223 > > > Repository: aurora > > > Description > ------- > > As always, feel free to tear it apart. > > I plan to add tests for the other commands as well, but wanted to get this > out sooner than later to ensure others agreed with my approach. > > > Diffs > ----- > > src/main/python/apache/aurora/admin/BUILD > 480dad69d8122168a6b7222ef9df0eb689fae386 > src/main/python/apache/aurora/admin/mesos_maintenance.py > d8ffdec062db07ce82556fb38ff4a5eea7921953 > src/main/python/apache/aurora/client/bin/BUILD > dbabfd0e56288d04c399e20976ea6e0287112d91 > src/main/python/apache/aurora/client/commands/BUILD > 14bbdd4d0cb0e8e5102e75baef4e8b34d98967c0 > src/main/python/apache/aurora/client/commands/admin.py > 989c5b625b48fe67ef1297ceda8d7e35cb8ead7e > src/main/python/apache/aurora/client/commands/maintenance.py PRE-CREATION > src/test/python/apache/aurora/admin/BUILD > 258b1eb58a4eec1650bfd317823f76a30889f912 > src/test/python/apache/aurora/admin/test_mesos_maintenance.py > 000fbe5a873669bf55a9110f1aa6f1d7c8350ef8 > src/test/python/apache/aurora/client/commands/BUILD > 6d448d7320a78bf140873b743805846c179281a7 > src/test/python/apache/aurora/client/commands/test_maintenance.py > PRE-CREATION > > Diff: https://reviews.apache.org/r/18334/diff/ > > > Testing > ------- > > ./pants ./src/test/python/apache/aurora:all > > particularly: > > src.test.python.apache.aurora.client.commands.maintenance > ..... SUCCESS > > > Thanks, > > Joe Smith > >
