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