> On Feb. 21, 2015, 12:53 a.m., Joshua Cohen wrote:
> > Can you fill in testing done?
> 
> Bill Farner wrote:
>     Honest question - do you find that useful for changes like this?  I find 
> it redundant to always type `./gradlew build -Pq`, especially since the build 
> bot will do that anyhow.

My take is that, yes, I trust that you ran the tests. But for someone who's 
pushing a review for the first time, who knows? And if they just pick a random 
commit from the log and look at the Testing Done section, I'd like for it to be 
consistent and easy to emulate.


> On Feb. 21, 2015, 12:53 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java, 
> > lines 199-200
> > <https://reviews.apache.org/r/31248/diff/1/?file=871335#file871335line199>
> >
> >     Use StringBuilder and String.format?
> 
> Bill Farner wrote:
>     See my reply to Maxim's comment.  javac is smart about using 
> StringBuilder behind the scenes in cases like this.  Do you find 
> String.format more readable in cases like this?  Personally i do not.

I've never had a problem with multiple concatenated variables, but my main 
concern is that we're consistent. My interpretation of our style based on 
review feedback I've gotten is that when concatenating more than one variable 
into a string we err on the side of using String.format for readability 
(perhaps we should actually fork the twitter style guide and codify these 
things?).


- Joshua


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31248/#review73353
-----------------------------------------------------------


On Feb. 21, 2015, 6:33 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31248/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2015, 6:33 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1028
>     https://issues.apache.org/jira/browse/AURORA-1028
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Export stats for source and reason for LOST tasks, and status delivery 
> latency.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 
> 1d8f0128732756db74576ee669f6a2718fecc105 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> ffc30bb548706df7bec9e1502242890e9b5eb942 
>   src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 
> 59ad9e65589c421cefb76f265446fa2885e6198c 
>   src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> d02c6b32841d5d39c5780e7a044079a38729effb 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31248/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to