On 02/09/2019 13:14, Severin Gehwolf wrote:
snip... >>> >> >> Most of this looks good. I was a little confused at first because the >> patch in your webrev looks quite different to the 11u changeset. >> However, once applied locally to the 8u repo, the diff between the two >> was as suggested and the PCSC library files (those in MUSCLE) were >> identical. I don't know what webrev did in creating that patch. >> >> The bit I don't understand is why you've decided to drop the copyright >> header change on the floor. Just because the original in 8u has 2014, >> while the original in 11u had 2015, does not make the change inapplicable. > > OK. I see. I wasn't sure how to deal with copyright year updates. I've > added the copyright update back. The patch is now identical to JDK 11u > (modulo differing copyright year base: 2014 - jdk 8 - vs. 2018 - jdk > 11): > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8218780/jdk8/02/webrev > This looks good now (though still issues with the original diff vs local application of it) Please flag the bug for approval. > >> A better alternative may actually be to backport JDK-8207233 [0] first >> which is a nice little cleanup patch. I suspect this patch would then >> apply cleanly as these PCSC files are rarely touched. >> >> [0] https://bugs.openjdk.java.net/browse/JDK-8207233 > > Hmm, this seems overkill just to get the copyright hunk to apply. I'd > prefer to keep this dependency out of scope for this patch. While a > nice clean-up it's not without risk backporting that too. Thoughts? > Well, I think it's a useful cleanup too, but I'm not going to force it as a dependency of this. I just thought it might make the review for this fix redundant as it would then be a clean backport. I might look into it myself at a later date. > Thanks, > Severin > -- Andrew :) Senior Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net) Fingerprint = 5132 579D D154 0ED2 3E04 C5A0 CFDA 0F9B 3596 4222 https://keybase.io/gnu_andrew
signature.asc
Description: OpenPGP digital signature
