if we don't use a wrapper we get PRs like https://github.com/apache/cloudstack/pull/2276 in the future, trying to update logging touches 1710 files. I think we should go for the wrapper model on these kind of utilities.
On Thu, Jan 11, 2018 at 9:59 PM, Rafael Weingärtner < rafaelweingart...@gmail.com> wrote: > Wrapping would still hold code on our side. We have to get rid of code… > > If we want to start removing CloudStack’s StringUtils in favor of > StringUtils from Apache, we could start creating PRs by components (java > project in Eclipse). That is manageable to do and to review. There are > about 119 classes that use CloudStack’s StringUtils. > > > We will not be able to remove CloudStack's StringUtils though. There are > very specific things there such as "applyPagination" that should not even > be there... I guess the programmer was running out of places to write code > > On Thu, Jan 11, 2018 at 6:25 PM, Daan Hoogland <daan.hoogl...@gmail.com> > wrote: > > > All, I am having second thoughts. I think we should maintain a wrapper > for > > string utils and pass through as much as possible to commons string > utils. > > A similar thing is applicable to logging. It was started at one time and > a > > second attempt was started to use slf4j. > > I think we should encapsulate these kind of utilities to facilitate > > migration. > > There is also json and xml formatting and maybe handling sockets and (big > > one) data access objects :/ > > > > @Ron, all string utils are static methods. > > > > On Thu, Jan 11, 2018 at 12:11 AM, Ron Wheeler > <rwheeler@artifact-software. > > com> wrote: > > > > > Certainly better to find the references and remove them if you can get > > > that done in a single effort. > > > > > > Just a technical question: Could one not just add the Warning to the > > > constructor? > > > Might have to create a null (log warning only) constructor. > > > > > > Ron > > > > > > > > > On 10/01/2018 3:58 PM, Daan Hoogland wrote: > > > > > >> We can add log messages to each of the methods in StringUtils but I do > > not > > >> think that is a good way to go. Any method you touch you can reform or > > >> remove anyhow. > > >> > > >> On Wed, Jan 10, 2018 at 9:51 PM, Ron Wheeler < > > >> rwhee...@artifact-software.com > > >> > > >>> wrote: > > >>> Agreed about deprecation. > > >>> A logged WARNing would be detected during testing as well as at > > run-time. > > >>> > > >>> Ron > > >>> > > >>> On 10/01/2018 3:34 PM, Daan Hoogland wrote: > > >>> > > >>> Ron, we could but that would only log during compile-time, not on > > >>> runtime. > > >>> I am doing some analysis and commenting in Wido's ticket. > > >>> > > >>> On Wed, Jan 10, 2018 at 9:23 PM, Ron Wheeler > > <rwheeler@artifact-software. > > >>> com> wrote: > > >>> > > >>> Is it possible to mark it as deprecated and have it log a warning > when > > >>>> used? > > >>>> > > >>>> Ron > > >>>> > > >>>> > > >>>> On 10/01/2018 2:26 PM, Daan Hoogland wrote: > > >>>> > > >>>> I think we could start with giving it an explicit non standard name > > like > > >>>>> CloudStackLocalStringUtils or something a little shorter. Making > sure > > >>>>> that > > >>>>> we prefer for these types of utils to be imported from other > > projects. > > >>>>> > > >>>>> On Wed, Jan 10, 2018 at 4:26 PM, Wido den Hollander < > w...@widodh.nl> > > >>>>> wrote: > > >>>>> > > >>>>> > > >>>>> On 01/10/2018 01:09 PM, Rafael Weingärtner wrote: > > >>>>>> > > >>>>>> Instead of creating a PR for that, we could do the bit by bit job > > >>>>>> > > >>>>>>> (hopefully one day we finish the job). > > >>>>>>> Every time we see a code using ACS's StringUtils, we check if it > > can > > >>>>>>> be > > >>>>>>> replaced by Apache's one. > > >>>>>>> > > >>>>>>> > > >>>>>>> Yes, but that will slip from peoples attention and we will > probably > > >>>>>>> see > > >>>>>>> > > >>>>>> cases where people still use the old one by accident. > > >>>>>> > > >>>>>> I've created a issue: https://issues.apache.org/jira > > >>>>>> /browse/CLOUDSTACK-10225 > > >>>>>> > > >>>>>> I also started on some low hanging fruit as some methods in > > >>>>>> StringUtils > > >>>>>> are not used or are very easy to replace. > > >>>>>> > > >>>>>> > > >>>>>> Wido > > >>>>>> > > >>>>>> On Wed, Jan 10, 2018 at 10:01 AM, Wido den Hollander < > > w...@widodh.nl> > > >>>>>> > > >>>>>> wrote: > > >>>>>>> > > >>>>>>> > > >>>>>>> On 01/10/2018 12:01 PM, Daan Hoogland wrote: > > >>>>>>> > > >>>>>>>> I'd say remove as much functionality as we can from 'our' > > >>>>>>>> StringUtils > > >>>>>>>> and > > >>>>>>>> > > >>>>>>>> phase them out asap. > > >>>>>>>>> > > >>>>>>>>> > > >>>>>>>>> Yes, but such a PR would be invasive and would be difficult to > > >>>>>>>>> merge > > >>>>>>>>> and > > >>>>>>>>> > > >>>>>>>>> also break a lot of other code. > > >>>>>>>> > > >>>>>>>> It's not easy since it will touch a lot, but I mean, a lot of > > files. > > >>>>>>>> > > >>>>>>>> Our StringUtils was a very good solution, but the Apache one is > > >>>>>>>> better I > > >>>>>>>> think. > > >>>>>>>> > > >>>>>>>> Wido > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> On Wed, Jan 10, 2018 at 11:59 AM, Wido den Hollander < > > >>>>>>>> w...@widodh.nl> > > >>>>>>>> > > >>>>>>>> wrote: > > >>>>>>>> > > >>>>>>>>> Hi, > > >>>>>>>>> > > >>>>>>>>> We have com.cloud.utils.StringUtils which has a few nice > > functions, > > >>>>>>>>> > > >>>>>>>>>> but > > >>>>>>>>>> throughout the code I also see org.apache.commons.lang.String > > >>>>>>>>>> Utils > > >>>>>>>>>> > > >>>>>>>>>> They both provide about the same functionality, but which one > do > > >>>>>>>>>> we > > >>>>>>>>>> prefer? > > >>>>>>>>>> > > >>>>>>>>>> I'd say org.apache.commons.lang.StringUtils as that allows us > > to > > >>>>>>>>>> remove > > >>>>>>>>>> our own StringUtils, but we could also have 'our' StringUtils > > >>>>>>>>>> simply > > >>>>>>>>>> be a > > >>>>>>>>>> wrapper around org.apache.commons.lang.StringUtils > > >>>>>>>>>> > > >>>>>>>>>> Opinions? > > >>>>>>>>>> > > >>>>>>>>>> Wido > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>> -- > > >>>> Ron Wheeler > > >>>> President > > >>>> Artifact Software Inc > > >>>> email: rwhee...@artifact-software.com > > >>>> skype: ronaldmwheeler > > >>>> phone: 866-970-2435, ext 102 > > >>>> > > >>>> > > >>>> > > >>> -- > > >>> Daan > > >>> > > >>> > > >>> -- > > >>> Ron Wheeler > > >>> President > > >>> Artifact Software Inc > > >>> email: rwhee...@artifact-software.com > > >>> skype: ronaldmwheeler > > >>> phone: 866-970-2435, ext 102 > > >>> > > >>> > > >>> > > >> > > > > > > -- > > > Ron Wheeler > > > President > > > Artifact Software Inc > > > email: rwhee...@artifact-software.com > > > skype: ronaldmwheeler > > > phone: 866-970-2435, ext 102 > > > > > > > > > > > > -- > > Daan > > > > > > -- > Rafael Weingärtner > -- Daan
