aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kevin Sweeney" <kevi...@apache.org>
Subject Re: Review Request 35498: Compute SLA stats for non-prod jobs
Date Fri, 26 Jun 2015 18:02:09 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message