Hi all,

I would like to ask someone with a reviewer status in jdk7u project to look at these changes.

thanks,
dmeetry

On 02/27/2014 05:44 PM, Joel Borggren-Franck wrote:
Hi,

I looked at webrev.1. Looks good.

cheers
/Joel

On 2014-02-25, dmeetry degrave wrote:
Thanks for looking at this, Peter!

On 02/24/2014 04:42 PM, Peter Levart wrote:
Hi Dmeetry,

On 02/22/2014 01:22 PM, dmeetry degrave wrote:
Hi all,

I would like to ask for a review of combined back port for
7u-dev/7u80. The main goal is to have a fix for 7122142 in jdk7, it
also integrates the changes from 8005232, 7185456, 8022721

https://bugs.openjdk.java.net/browse/JDK-7122142
https://bugs.openjdk.java.net/browse/JDK-8005232
https://bugs.openjdk.java.net/browse/JDK-7185456
https://bugs.openjdk.java.net/browse/JDK-8022721

Original jdk8 changes:

7122142: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e4ce6502eac0
8005232: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1109bfff4e92
7185456: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ae03282ba501
8022721: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2281a7f79738

back port:

http://cr.openjdk.java.net/~dmeetry/7122142.8005232.7185456.8022721/webrev.0/


Patches can't be applied cleanly, hence it was a manual back port,
though the final result is equivalent to applying the patches in
chronological order (8005232, 7185456, 7122142, 8022721) and applying
all the relevant rejected parts

It's good to see those patches being back-ported to 7u. By browsing the
webrev, I don't see any obvious difference between the original patches
and the backport.

there shouldn't be any!

Do you happen to remember in what part of code there
were rejects so that you had to manually apply the changes?

there were conflicts due to small difference between 7 and 8
(copyrights, white spaces, @SuppressWarnings, Class<?>,...).

I copied all rejected parts and original patches here:

http://cr.openjdk.java.net/~dmeetry/7122142.8005232.7185456.8022721/webrev.1/rej/

(with one exception, AnnotationTypeRuntimeAssumptionTest.java test was
not included due to jdk8 API).

Ah, It's the Class.getDeclaredAnnotation(Class) that's new in JDK8.
Here's the changed test that only uses the JDK7 API so you can include
this test too:

http://cr.openjdk.java.net/~plevart/jdk7u/7122142/AnnotationTypeRuntimeAssumptionTest.java

Thanks!

http://cr.openjdk.java.net/~dmeetry/7122142.8005232.7185456.8022721/webrev.1/

(just with the new test added).

thanks,
dmeetry

All tests in test/java/lang/annotation passed.

thanks,
dmeetry

Regards, Peter

Reply via email to