Hi David, Thanks for reviewing and sponsoring!
Best, Götz > Am 21.08.2017 um 04:00 schrieb David Holmes <david.hol...@oracle.com>: > > Hi Goetz, > >> On 18/08/2017 4:08 PM, Lindenmaier, Goetz wrote: >> Hi, >> Thanks Volker! >> Could I please get a second review? I also need a sponsor please. >> http://cr.openjdk.java.net/~goetz/wr17/8185112-macLocale/webrev.02/ > > This seems fine to me and I can sponsor it. But as Arno is the contributor > wouldn't you also be a reviewer? ;-) I'll add you anyway. > > Cheers, > David > >> Thanks, >> Goetz. >>> -----Original Message----- >>> From: Volker Simonis [mailto:volker.simo...@gmail.com] >>> Sent: Donnerstag, 17. August 2017 19:33 >>> To: Lindenmaier, Goetz <goetz.lindenma...@sap.com> >>> Cc: serviceability-dev (serviceability-dev@openjdk.java.net) >>> <serviceability- >>> d...@openjdk.java.net>; Zeller, Arno <arno.zel...@sap.com> >>> Subject: Re: RFR(XS): 8185112: [TESTBUG] Servicability tests cannot parse >>> float >>> if non US locale. >>> >>> Thanks Goetz - looks good now! >>> >>> And sorry for the delay :) >>> Volker >>> >>> >>> On Thu, Aug 3, 2017 at 11:30 AM, Lindenmaier, Goetz >>> <goetz.lindenma...@sap.com> wrote: >>>> Hi Volker, >>>> >>>>> OK, I don't want to unnecessarily block this change but IMO if we >>>>> don't need the integer parsing function at all we should remove it or >>>>> otherwise change it to use NumberFormat as well. >>>> We need the integer parsing function. But the intergers parsed >>>> are not printed with formatting. Just plain 123456. So I don't see >>>> how the locale will interfere. >>>> But I changed it anyways (took a while because the tests didn't run >>>> nightly): >>>> http://cr.openjdk.java.net/~goetz/wr17/8185112-macLocale/webrev.02/ >>>> >>>> Best regards, >>>> Goetz. >>>> >>>>> -----Original Message----- >>>>> From: Volker Simonis [mailto:volker.simo...@gmail.com] >>>>> Sent: Freitag, 28. Juli 2017 16:45 >>>>> To: Lindenmaier, Goetz <goetz.lindenma...@sap.com> >>>>> Cc: serviceability-dev (serviceability-dev@openjdk.java.net) >>>>> <serviceability- >>>>> d...@openjdk.java.net>; Zeller, Arno <arno.zel...@sap.com> >>>>> Subject: Re: RFR(XS): 8185112: [TESTBUG] Servicability tests cannot parse >>>>> float if non US locale. >>>>> >>>>> On Fri, Jul 28, 2017 at 9:30 AM, Lindenmaier, Goetz >>>>> <goetz.lindenma...@sap.com> wrote: >>>>>> Hi Volker, >>>>>> >>>>>> thanks for looking at this change! >>>>>> >>>>>>> Looks good, but don't we also need this for getIntValue() as well? >>>>>> This class is a helper class for testing jstat. To my >>>>>> knowledge jstat never formats integers, so the >>>>>> current parsing should cover all possible outputs >>>>>> to be tested. >>>>>> >>>>> >>>>> OK, I don't want to unnecessarily block this change but IMO if we >>>>> don't need the integer parsing function at all we should remove it or >>>>> otherwise change it to use NumberFormat as well. >>>>> >>>>> Regards, >>>>> Volker >>>>> >>>>>> Best regards, >>>>>> Goetz. >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: Volker Simonis [mailto:volker.simo...@gmail.com] >>>>>>> Sent: Thursday, July 27, 2017 11:49 AM >>>>>>> To: Lindenmaier, Goetz <goetz.lindenma...@sap.com> >>>>>>> Cc: serviceability-dev (serviceability-dev@openjdk.java.net) >>>>> <serviceability- >>>>>>> d...@openjdk.java.net>; Zeller, Arno <arno.zel...@sap.com> >>>>>>> Subject: Re: RFR(XS): 8185112: [TESTBUG] Servicability tests cannot >>>>>>> parse >>>>>>> float if non US locale. >>>>>>> >>>>>>> Looks good, but don't we also need this for getIntValue() as well? >>>>>>> I.e. can't an integer be "1.234.678" (German style) as well as >>>>>>> "1,234,678" (American style) for example ? >>>>>>> >>>>>>> Thanks, >>>>>>> Volker >>>>>>> >>>>>>> On Mon, Jul 24, 2017 at 9:07 AM, Lindenmaier, Goetz >>>>>>> <goetz.lindenma...@sap.com> wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Some tests use Float.valueOf for String to float converting. If an >>>>>>>> other >>>>>>>> locale than US is used the test failed. We observed this on Mac. >>>>>>>> Changed to use NumberFormat to work with all locales. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Please review this change. I please need a sponsor. >>>>>>>> >>>>>>>> http://cr.openjdk.java.net/~goetz/wr17/8185112- >>>>> macLocale/webrev.01/ >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Best regards, >>>>>>>> >>>>>>>> Goetz