RFR 8235699 : ArrayIndexOutOfBoundsException in CalendarBuilder.toString

[Volker Simonis]

Looks good to me as well!

I'll sponsor this and push it.

Best regards,
Volker

On Wed, Feb 5, 2020 at 6:47 AM Roger Riggs <Roger.Riggs at oracle.com> wrote:
>
> Looks good,  Thanks
>
> On 2/4/20 8:06 PM, Verghese, Clive wrote:
> > Thank you for the quick feedback.
> >
> > I have updated the Webrev
> > http://cr.openjdk.java.net/~alvdavi/webrevs/8235699/webrev.8u.jdk.06/
> >
> > Regards,
> > Clive Verghese
> >
> > On 2/4/20, 4:42 PM, "core-libs-dev on behalf of Roger Riggs" <core-libs-dev-bounces at openjdk.java.net on behalf of roger.riggs at oracle.com> wrote:
> >
> >      Hi Clive,
> >
> >      To clarify about the Oracle copyright, it should be included only if the
> >      file was
> >      derived from Oracle copyright source. The previous copyright is fine.
> >
> >      Sorry for any confusion.
> >
> >      Regards, Roger
> >
> >      On 2/4/20 5:20 PM, Roger Riggs wrote:
> >      > Hi Clive,
> >      >
> >      > The update looks good to me.
> >      >
> >      > Thanks, Roger
> >      >
> >      >
> >      > On 2/4/20 5:17 PM, Verghese, Clive wrote:
> >      >> Hi Roger,
> >      >>
> >      >> Thank you for the feedback. I have addressed your comments and
> >      >> updated the Webrev.
> >      >> http://cr.openjdk.java.net/~alvdavi/webrevs/8235699/
> >      >>
> >      >> Regards,
> >      >> Clive Verghese
> >      >>
> >      >>
> >      >> On 2/4/20, 12:34 PM, "core-libs-dev on behalf of Roger Riggs"
> >      >> <core-libs-dev-bounces at openjdk.java.net on behalf of
> >      >> Roger.Riggs at oracle.com> wrote:
> >      >>
> >      >>      Hi Clive,
> >      >>           A few comments:
> >      >>           The copyrights for the new files need to follow the
> >      >> template in
> >      >>      <repo>make/template/gpl-header.
> >      >>      In particular, it needs to include Oracle as a copyright holder.
> >      >>           bug235699.java:
> >      >>           The @summary should be more informative; "it works" tells
> >      >> nothing about
> >      >>      the case being tested.
> >      >>           Please rename bug8235699.java to Bug8235699.java;
> >      >>      There are more initial upper case tests than lower and it would
> >      >> be good
> >      >>      to converge over time.
> >      >>           35: A comment saying that a AIOOBE should not occur would
> >      >> be helpful.
> >      >>           CalendarBuilderTest.java:
> >      >>           Should describe the condition that is being created.
> >      >>      27:  "Test that CalendarBuilder.toString does not produce IOOBE"
> >      >>           Are all of the assignments necessary to cause the bug?
> >      >>      Remove any that are not; they are misleading.  (onlysetting the
> >      >> year is
> >      >>      needed)
> >      >>           Thanks, Roger
> >      >>                On 1/2/20 4:18 PM, Verghese, Clive wrote:
> >      >>      > Hi Alan,
> >      >>      >
> >      >>      > Thanks for the feedback,
> >      >>      >
> >      >>      > I have removed the @Author tag and updated the tests as per
> >      >> your recommendation.
> >      >>      >
> >      >>      > Updated Webrev
> >      >>      > http://cr.openjdk.java.net/~phh/8235699/webrev.04/
> >      >>      >
> >      >>      > Regards,
> >      >>      > Clive Verghsese
> >      >>      >
> >      >>      > Regards,
> >      >>      > Clive Verghese
> >      >>      >
> >      >>      > On 1/2/20, 11:19 AM, "Volker Simonis" <simonisv at amazon.de> wrote:
> >      >>      >
> >      >>      >      On 02.01.20 18:39, Alan Bateman wrote:
> >      >>      >      > On 02/01/2020 13:26, Volker Simonis wrote:
> >      >>      >      >> :
> >      >>      >      >>
> >      >> http://cr.openjdk.java.net/~simonis/webrevs/2020/8235699.02/
> >      >>      >      >>
> >      >>      >      >> Ready to push?
> >      >>      >      >>
> >      >>      >      > You shouldn't need to use core reflection here. Instead
> >      >> you can create
> >      >>      >      > the test so that it is compiled and runs as if part of
> >      >> the java.text
> >      >>      >      > package, e.g.
> >      >>      >      >
> >      >>      >      > @build java.base/java.text.CalendarBuilderToStringTest
> >      >>      >      > @main Driver
> >      >>      >      >
> >      >>      >
> >      >>      >      Thanks for the hint. I wasn't aware of this possibility.
> >      >>      >      I think Clive will rewrite the test.
> >      >>      >
> >      >>      >      > Do you really want the @author tag? We try to avoid
> >      >> them if possible
> >      >>      >      > because they are so hard to remove, even when
> >      >> code/tests are changed
> >      >>      >      > significantly.
> >      >>      >
> >      >>      >      No not really. It was just a part of the template I used :)
> >      >>      >      @Clive: please feel free to remove the author tag.
> >      >>      >
> >      >>      >      > -Alan
> >      >>      >
> >      >>      >
> >      >>      >
> >      >>      >
> >      >>
> >      >
> >
> >
> >
>