> On July 22, 2015, 7:36 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/app/VolumeParser.java, line 45 > > <https://reviews.apache.org/r/36703/diff/1/?file=1019008#file1019008line45> > > > > 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.
+1 to this -1. The straight Joiner.on -> String.join changes are great, but the other changes really obfuscate what's going on. - Joshua ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36703/#review92635 ----------------------------------------------------------- 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 > >
