code review request : 8003147 port fix for BCEL bug 39695

[David Buck]

Hi Joe!

Thank you for looking at this.

 > You didn't apply the original Apache completely. Was the omission of the
 > change in toString() method intentional?

Yes, the improvement in toString() was not related to the issue and 
seems to have been included in the apache version of this fix on a whim. 
I did not feel it was appropriate to include it in the port as it is 
clearly outside of the scope of BCEL bug 39695.

 > I see that you've sent your test to Patrick. Have you run regression
 > test for this patch?

I have only tested the different versions of my own testcase. I do not 
know anything about the test cases that Patrick is maintaining. If you 
or Patrick can walk me through how to get and run the test cases SQE 
uses, I will be more than happy to do that. Or if Patrick or you prefer, 
I can send you my build if it would be easier for one of you to run the 
tests yourself.

Cheers,
-Buck

On 2012/12/13 4:14, Joe Wang wrote:
> Hi David,
>
> You didn't apply the original Apache completely. Was the omission of the
> change in toString() method intentional?
>
> I see that you've sent your test to Patrick. Have you run regression
> test for this patch?
>
> Thanks,
> Joe
>
> On 12/9/2012 10:25 PM, David Buck wrote:
>> Hi!
>>
>> I would like to request a code review of my JDK8 fix for the following
>> issue:
>>
>> [ 8003147 : port fix for BCEL bug 39695 to our copy bundled as part of
>> jaxp ]
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8003147
>>
>> In addition to the fix that the BCEL project had for this issue, I
>> needed to "port" some of their support for LocalVariableTypeTable:s to
>> our copy of BCEL as well. Implementing support for LVTT is very
>> straightforward and does not add much to the complexity or risk of
>> this change (especially considering the limited scope of jaxp's use of
>> BCEL).
>>
>> Here is the webrev for my fix:
>>
>> [ Code Review for jaxp ]
>> http://cr.openjdk.java.net/~dbuck/8003147/webrev.00/
>>
>> My understanding is that the test cases for our copy of the jaxp code
>> are handled in a separate repository / test suite. I have been in
>> contact with Patrick Zhang (Oracle QA) and Joe Wang and have provided
>> a junit test for this issue as requested. Please see bug report for a
>> simpler (non-junit) test-case. If for some reason anyone wants to see
>> the junit-based test, please let me know and I can provide a copy of
>> that as well.
>>
>> Best Regards,
>> -Buck