On 12/12/2012 8:37 PM, David Buck wrote:
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.

Ok.


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

You may ask Patrick for instructions on how to run regression test. It needs to be run separately since they are not in the jdk repo yet.

Thanks,
Joe


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

Reply via email to