On 1/16/20 9:50 AM, Remi Forax wrote:
----- Mail original -----
De: "Joe Wang" <huizhe.w...@oracle.com>
À: "Daniel Fuchs" <daniel.fu...@oracle.com>, "core-libs-dev" 
<core-libs-dev@openjdk.java.net>
Envoyé: Jeudi 16 Janvier 2020 18:40:18
Objet: Re: RFR [15/java.xml] 8235368 : Update BCEL to Version 6.4.1
On 1/16/20 2:35 AM, Daniel Fuchs wrote:
Hi Joe,

Looks OK to me as well.
Thanks for the review!

I am a bit surprised by the number of methods that are no longer
`final` though. Do you know what was the motivation for those
changes?
The original patch did not have any detailed comment or link to a bug
report. The title was "Remove redundant modifiers. Minor Javadoc and
formatting. "  So it seemed they were cleaning up "redundant modifiers".
It would be interesting if that's the reason to remove 'final'. However,
it has no impact on our usage of the library in java.xml.
It's because the class itself is declared final (at least on the few files I've 
taken a look), so final on a method is redundant.

Aha, indeed! I double-checked classes such as Code.java, CodeException.java, etc.


Meanwhile, I noticed I missed the new classes in the webrev. I used a changelist to create webrevs and forgot to add the new ones to the list. Sorry about that. Here's the updated webrev:

http://cr.openjdk.java.net/~joehw/jdk15/8235368/webrev_02/

Thanks,
Joe


Best regards,
Joe
regards,
Rémi

best regards,

-- daniel

On 14/01/2020 20:08, Joe Wang wrote:
Hi,

Please review an update to BCEL 6.4.1.

JBS: https://bugs.openjdk.java.net/browse/JDK-8235368
webrev:
http://cr.openjdk.java.net/~joehw/jdk15/8235368/webrev/index.html

Similar approach as the last update:
1. Format
      All format changes are kept as they are in the source in order
to reduce the amount of changes in future updates, the exceptions are
extreme long lines.

2. Exclusions
      Contents that were not in the JDK or unnecessary for java.xml
are excluded. This includes: the ability to load arbitrary classes
and classes related to ClassLoader, ClassPath and JavaWrapper, and
relevant methods and references in other classes; System Properties
used to set cache sizes and track certain statistics (caches are set
as in previous version); Deprecated classes and related contents.

3. Warnings
      Warnings were the main reason for the changes made to the
original source. It has been done in the previous update. They are
re-applied for this update. The LastModified field indicates such
changes to the original source.

4. Deprecated fields to private and references to deprecated methods
     Deprecated fields in the original source were changed to private
ones in previous update. References to deprecated methods were
modified to use proper methods. These changes are inherited in this
update.

5. Test
      Since the update does not affect java.xml's usage of the BCEL
component, it is essential to pass all of the existing tests. I've
run the tests multiple times and noted that all of the XML functional
and unit tests passed, so were JCK XML tests. A performance test is
running.


Thanks,
Joe



Reply via email to