Imteyaz,

I am on your pull request. I drop one comment, would you mind to take a
look?

Thanks,
-Ian.


On Sat, Jan 19, 2019 at 3:58 AM Imteyaz Khan <[email protected]> wrote:

> Hi All,
>    PR which I raised mentioned in email thread, due to recent changes it
> encounters some conflicts and after resolving and taking latest pull it was
> showing 500+ file chages (It might I have not done it right way :). To
> avoide confusion I have closed existing one and created new on. This new
> *PR
> <https://github.com/apache/incubator-dubbo/pull/3274>*has all the changes
> till date which I have incorporated as review feedback.
>
> In my old PR <https://github.com/apache/incubator-dubbo/pull/3090>there
> was
> request changes asked by @zonghaishang <https://github.com/zonghaishang>
> (Not
> thread safe implementation), which I beleive I have addressed as there was
> also about making thread safe and reducing the creation of
> SimpleDateFormat.
>
> Old PR link : https://github.com/apache/incubator-dubbo/pull/3090
> New PR link :https://github.com/apache/incubator-dubbo/pull/3274
>
> Once again sorry for the inconvenience, and would request review my PR.
>
> Regards
> Imteyaz
>
> On Sat, Dec 29, 2018 at 11:58 AM yuhang xiu <[email protected]> wrote:
>
> > Hi, @beiwei @khan
> >
> > There have been a lot of things in recent work.
> > After the end of New Year's Day, I believe that there will be more time
> to
> > work in the community, I am very sorry.
> >
> > I will dispose of this PR next week. @khan
> > :)
> >
> > Ian Luo <[email protected]> 于2018年12月29日周六 上午11:14写道:
> >
> > > I took a glance at the latest change. It looks good to me since we
> avoid
> > of
> > > using thread-local and new instance for every incoming request, instead
> > one
> > > singleton date formatter runs in one single thread, which I totally
> agree
> > > with.
> > >
> > > Let's wait for Yuhang's review comments :)
> > >
> > > -Ian.
> > >
> > >
> > > On Sat, Dec 29, 2018 at 12:11 AM Imteyaz Khan <[email protected]>
> > > wrote:
> > >
> > > > Due to my own confusion and bad I found I found my PR is containing
> > more
> > > > commits and more file modification other than mine. So avoid the
> > > confusion
> > > > I created new PR and close the old one. This PR containing the all
> the
> > > > review comments from old PR. Please review mine this PR
> > > > <https://github.com/apache/incubator-dubbo/pull/3090>
> > > >
> > > >
> > > > https://github.com/apache/incubator-dubbo/pull/3090
> > > >
> > > > Sorry for the inconvenience caused.
> > > >
> > > >
> > > > On Thu, Dec 27, 2018 at 3:24 PM yuhang xiu <[email protected]>
> wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > > I simply checked your pr and I agree with you. I will review it.
> > > > > Nice work!
> > > > >
> > > > > Imteyaz Khan <[email protected]> 于2018年12月27日周四 下午5:35写道:
> > > > >
> > > > > > Hi All,
> > > > > >   I have raised a separate PR for the same issue 3026.
> > > > > >
> > > > > > https://github.com/apache/incubator-dubbo/pull/3080
> > > > > >
> > > > > > As my old PR was containing may commits of others and reviewer
> > would
> > > > like
> > > > > > me to clear those commits and I tried hard to achieve the same
> but
> > > > failed
> > > > > > so landed up creating new PR
> > > > > > <https://github.com/apache/incubator-dubbo/pull/3080>.
> > > > > >
> > > > > > Please note :: older PR
> > > > > > <https://github.com/apache/incubator-dubbo/pull/3027> 3027 has
> > been
> > > > > closed
> > > > > > but all the comments are applicable here.
> > > > > >
> > > > > > On Thu, Dec 20, 2018 at 5:35 PM Imteyaz Khan <
> > [email protected]
> > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hi All,
> > > > > > >   I have raised a PR <
> > > > > > https://github.com/apache/incubator-dubbo/pull/3027>for
> > > > > > > issue 3026 <
> > https://github.com/apache/incubator-dubbo/issues/3026
> > > >.
> > > > In
> > > > > > > this PR , I am reducing the  number of SimpleDateFormat object
> > > > creation
> > > > > > > from each log message to per thread wise and reusing it. Please
> > > > review
> > > > > > it.
> > > > > > >
> > > > > > > https://github.com/apache/incubator-dubbo/pull/3027
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Reply via email to