Ok...please resubmit. --Alex
> -----Original Message----- > From: Laszlo Hornyak [mailto:nore...@reviews.apache.org] On Behalf Of > Laszlo Hornyak > Sent: Tuesday, June 11, 2013 11:48 AM > To: cloudstack; Laszlo Hornyak; Alex Huang > Subject: Re: Review Request: use commons-lang StringUtils > > > > > 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/NfsSecondaryStorage > Resource.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 > > > >
