Well, there is always other approaches...If we did not use those static loggers, this number could be greatly reduced. Most of those objects are singletons and we could use a protected attribute in the first element of the hierarchy.
I do not mind a PR with this number of files changes as long as you stick to a single change, what I mind is the combination of high number of files and commits.Then, at least for me, it becomes pretty hard to track down things. On Fri, Jan 12, 2018 at 6:19 AM, Daan Hoogland <daan.hoogl...@gmail.com> wrote: > 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 > -- Rafael Weingärtner
