> On June 25, 2015, 10:59 a.m., Kevin Sweeney wrote: > > src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java, lines > > 138-139 > > <https://reviews.apache.org/r/35498/diff/7/?file=991483#file991483line138> > > > > Add getters for these fields and access them below via the getters > > rather than direct field access. > > Stephan Erb wrote: > I can submit an updated patch tonight. I've somewhat expected that some > of you would point it out :-) > > I thought about adding these when writing the patch but then decided > against them: Getters for final attributes on an inner class did not seem to > offer any meaningful encapsulation or help during refactoring.
In this case the class in question uses getters and direct field access inconsistently, I'd also be okay with removing all getters on that class and replacing them with direct field access. I think getters are more readable though (it's a code review red flag when I see a new direct field access). Other committers feel free to chime in here. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35498/#review89394 ----------------------------------------------------------- On June 26, 2015, 10:45 a.m., Stephan Erb wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35498/ > ----------------------------------------------------------- > > (Updated June 26, 2015, 10:45 a.m.) > > > Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. > > > Bugs: AURORA-1350 > https://issues.apache.org/jira/browse/AURORA-1350 > > > Repository: aurora > > > Description > ------- > > Compute SLA stats for non-prod jobs > > This is a first iteration closely following the design proposal of Maxim as > posted on the mailinglist. Feedback welcome. > > > Diffs > ----- > > NEWS a17f0e7c08fd30a0b2db6814a1c755111307228b > docs/sla.md 14e9108fda91200bbf56384c96b9cd926689311f > src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java > 82f36d5ca15df18bdc8ebbbd868d3394db38e603 > src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java > ff73ca6265bd0699791da5e5b6ed4aab9156d9e4 > src/main/java/org/apache/aurora/scheduler/sla/SlaModule.java > 64e986fb2e0f955dd4a9c7824eac9494728bf24e > src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java > cb98834e925793fc116814371548a30470830164 > src/test/java/org/apache/aurora/scheduler/sla/SlaModuleTest.java > 5ee123a03e3c8670e0c03b05c48a9f4c66f6af9d > > Diff: https://reviews.apache.org/r/35498/diff/ > > > Testing > ------- > > `./gradlew -Pq build` and a manual verification in Vagrant. > > > Thanks, > > Stephan Erb > >
