RFR 8235699 : ArrayIndexOutOfBoundsException in CalendarBuilder.toString

[Roger Riggs]

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
>      >>      >
>      >>      >
>      >>      >
>      >>      >
>      >>
>      >
>      
>      
>