> On June 10, 2013, 11:56 p.m., Alex Huang wrote: > > The patch did not apply cleanly. Please resubmit. Since you have to > > resubmit anyways, I think why not just change all of the code to use the > > commons.lang version. I see no point in keeping the method in StringUtils > > if it's replaceable by one in a standard library. > >
Hi Alex, I think it makes sense to keep the StringUtils.join(String, Object...) method because it allows to use varargs, unlike the one in commons-lang, and the code somewhat builds on it. - Laszlo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11767/#review21678 ----------------------------------------------------------- On June 9, 2013, 7:47 p.m., Laszlo Hornyak wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11767/ > ----------------------------------------------------------- > > (Updated June 9, 2013, 7:47 p.m.) > > > Review request for cloudstack. > > > Description > ------- > > commons-lang is already a transitive dependency of the utils project, which > allows removing some duplicated functionality. > This patch replaces StringUtils.join(String, Object...) with it's > commons-lang counterpart. > It also replaces calls to String join(Iterable<? extends Object>, String) in > cases where an array is already exist and it is only wrapped into a List. > > > Diffs > ----- > > server/src/com/cloud/storage/s3/S3ManagerImpl.java 61e5573 > > services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java > e7fa5b2 > utils/src/com/cloud/utils/S3Utils.java b7273a1 > utils/src/com/cloud/utils/StringUtils.java 14ff4b1 > utils/test/com/cloud/utils/StringUtilsTest.java 3c162c7 > > Diff: https://reviews.apache.org/r/11767/diff/ > > > Testing > ------- > > - Unit test added > > > Thanks, > > Laszlo Hornyak > >
