RFR 8235699 : ArrayIndexOutOfBoundsException in CalendarBuilder.toString

Verghese, Clive verghese at amazon.com
Wed Feb 5 01:06:21 UTC 2020


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



More information about the core-libs-dev mailing list