RFR 8235699 : ArrayIndexOutOfBoundsException in CalendarBuilder.toString

[Verghese, Clive]

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