----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18334/#review35362 -----------------------------------------------------------
src/main/python/apache/aurora/admin/mesos_maintenance.py <https://reviews.apache.org/r/18334/#comment65844> Inclined to revert this - better to explicitly call out a dependency on system time IMO. src/main/python/apache/aurora/client/commands/maintenance.py <https://reviews.apache.org/r/18334/#comment65839> DRY - these options are used multiple times. Consider factoring them as constants. src/test/python/apache/aurora/client/commands/test_maintenance.py <https://reviews.apache.org/r/18334/#comment65838> make_mock_options would be more descriptive. also docstring is not informative, consider dropping it. src/test/python/apache/aurora/client/commands/test_maintenance.py <https://reviews.apache.org/r/18334/#comment65836> parens aren't necessary src/test/python/apache/aurora/client/commands/test_maintenance.py <https://reviews.apache.org/r/18334/#comment65835> parens not necessary src/test/python/apache/aurora/client/commands/test_maintenance.py <https://reviews.apache.org/r/18334/#comment65837> no parens needed. src/test/python/apache/aurora/client/commands/test_maintenance.py <https://reviews.apache.org/r/18334/#comment65843> 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. - Kevin Sweeney 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 > >
