> On Dec. 30, 2015, 4:14 p.m., John Sirois wrote: > > I'd be happy with these commits landing as they come, but I'll recommend > > that be paired with a boilerplate comment referencing a tracking jira issue > > that explains the odd interfaces appearing during the transition.
+1 to landing commits as they come. I think it is preferable to landing several commits all at once at the end. - Zameer ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41804/#review112363 ----------------------------------------------------------- On Dec. 30, 2015, 4:03 p.m., Bill Farner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41804/ > ----------------------------------------------------------- > > (Updated Dec. 30, 2015, 4:03 p.m.) > > > Review request for Aurora, John Sirois and Zameer Manji. > > > Repository: aurora > > > Description > ------- > > This begins to define a proposed replacement args API, from the perspective > of the code consuming args. Args will be defined in interfaces, which the > eventual arg system will be responsible for implementing on the fly > (mechanism TBD). So while what is seen here is a large net increase in code, > the eventual conclusion will be roughly equivalent in terms of lines of code > in `Module`s. > > There are a few goals with the replacement: > - sidestep current development hurdles we have encountered with the args > system (intellij/gradle not working nicely with apt) > - leverage a well-maintained third-party argument parsing library > - encourage better testability of Module classes by always injecting all args > - enable user-friendly features like logical option groups for better > help/usage output > - stretch: enable alternative configuration inputs like a configuration file > or environment variables > > The rough plan of action is as follows (if the proposal looks good): > 1. repeat this patch for all other `@CmdLine` declaration sites (28 files) > 2. introduce a 'boot' `Injector` that is loaded with bindings for these > params implementations (i.e. centralize the `new ExecutorModuleParams() { .. > }` boilerplate you see here > 3. replace the args backend > (a) remove `@CmdLine Arg<>` declarations, moving help text to annotations > on interface methods > (b) implement a system to inject proxies that implement params classes > based on arg values, and binds them for injection > > Given that this is a large multi-stage effort, i may opt to implement it on a > branch and land it all at once in a stream of commits to avoid > churn/confusion in the meantime. Open to thoughts on that. > > > Diffs > ----- > > > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java > 949c299bdbc54f976db994266fb97f3099256f13 > > Diff: https://reviews.apache.org/r/41804/diff/ > > > Testing > ------- > > > Thanks, > > Bill Farner > >
