+1 Good catch Naoto
> On Mar 30, 2020, at 6:44 PM, Joe Wang <huizhe.w...@oracle.com> wrote: > > > On 3/30/20 3:12 PM, naoto.s...@oracle.com wrote: >> Hi Joe, >> >> Much better and cleaner! One nit is that you could remove "docLocator != >> null &&" as instanceof checks null. > > Indeed, null check is removed: > http://cr.openjdk.java.net/~joehw/jdk15/8238183/webrev_03/ > > Thanks, > Joe > >> >> Naoto >> >> On 3/30/20 2:56 PM, Joe Wang 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>
