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



src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java (line 45)
<https://reviews.apache.org/r/36703/#comment146858>

    Yeah, big -1 to me on changes like this.  The previous code was clear and 
concise.  The patch is a readability regression IMHO with no obvious upside.


- Bill Farner


On July 22, 2015, 7:30 p.m., Kevin Sweeney wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36703/
> -----------------------------------------------------------
> 
> (Updated July 22, 2015, 7:30 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Remove unnecessary use of Guava Joiner.
> 
> This replaces a Guava-ism with the JDK8 standard library APIs. Most of the 
> changes to use `String::join` instead of `Joiner.on(...).join(...)` should be 
> noncontroversial; however in some cases the resulting code is more verbose. 
> This is mostly due to the fact that the standard library requires that 
> arguments be instances of `CharSequence`, requiring the caller to map over 
> `Object::toString`. I'd argue minimizing use of Guava-isms when a standard 
> lib alternative exists is preferable in these cases but don't feel strongly 
> about it.
> 
> There are also some opportunistic changes to surrounding code to use lambdas, 
> method references, and the standard `Streams` API instead of `FluentIterable` 
> and `Ordering`.
> 
> `Joiner` is still used in its `withKeyValueSeparator` form, as no standard 
> library alternative exists for it.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java 
> 2b7613741a729e7065bbe74690d543b45802c400 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> b4163435ea337a9976fae2f84850af0320ab9884 
>   src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
> a5ffa5e95b301e536a84acf02817ea0c080559d0 
>   src/main/java/org/apache/aurora/scheduler/cron/CrontabEntry.java 
> 904dd72dbb9036d8c06353a953bf35b8f04cfdbc 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/Quartz.java 
> 45e062d3acbd7b8565bd3773c1e994aae96378e0 
>   src/main/java/org/apache/aurora/scheduler/http/CorsFilter.java 
> e413ad9bdccc329777b3d0764ba8684539956679 
>   src/main/java/org/apache/aurora/scheduler/http/Slaves.java 
> b64e18c4d5278e6cdb2e480043a45e6ec4f87484 
>   src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 
> 63c31eeb293771b6cc0a3e4cc62dd3a94853e727 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParser.java
>  ff8063c050e13b4bffda2661a817fdc023b80867 
>   
> src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java 
> 7232f603e21bbc9dbb5e05aedd5e493de519158e 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> 92c970c34ca9dc4f052760e5a3d3770a089d9a67 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
> 7c8f8b9b8d7deb082edc0f85a6d3da1536735545 
> 
> Diff: https://reviews.apache.org/r/36703/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Reply via email to