HI Joe, This version looks OK
Best Lance > On Mar 30, 2020, at 5:56 PM, Joe Wang <huizhe.w...@oracle.com> wrote: > > Hi Naoto, > > Thanks for the review! > > I refactored the code around getting the XML version and encoding from the > locator to get rid of the CCE. The impls with EventWriter and StreamWriter > share a base class. All three classes are therefore refactored. No material > change to the EventWriter impl. Two fields, xmlVersion and encoding, are > added since I expect they will be needed later when we work on fixing the > declaration. Note that, as of the current impl, encoding is not referenced in > the StreamWriter impl, which is part of the issue in transforming the > declaration (JDK-8241711). > > Here's webrev version 2: > http://cr.openjdk.java.net/~joehw/jdk15/8238183/webrev_02/ > > -Joe > > On 3/30/20 11:23 AM, naoto.s...@oracle.com wrote: >> Hi Joe, >> >> Can writeStartDocument() be simplified by checking "docLocator instanceof >> Locator2"? This way it won't need to catch CCE and issue no-arg version. >> >> Otherwise looks good to me. >> >> Naoto >> >> On 3/30/20 11:02 AM, Joe Wang wrote: >>> Hi, >>> >>> Please review a fix for the StAXResult impl. The issue was that it output >>> comment prior to the declaration, resulting in an invalid XML document. The >>> patch focuses on fixing this issue, but it does not cover other issues the >>> StAXResult impl may have (e.g. JDK-8241711). >>> >>> JBS: https://bugs.openjdk.java.net/browse/JDK-8238183 >>> webrev: http://cr.openjdk.java.net/~joehw/jdk15/8238183/webrev/ >>> >>> Thanks, >>> Joe >>> > <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
