Thanks Roger,
On 16/11/16 16:03, Roger Riggs wrote:
Hi Daniel,
DocumentBuilderFactory and classes:
- The new methods uses "Creates" instead of "Obtain";
perhaps it should be consistent with existing newInstance methods?
I had some pre-review advice from Joe (Wang) about who suggested
using the 3rd person (Obtains) rather than the imperative (Obtain).
Also I noticed that some factories use Get, some Obtain, some Create...
Because this is a new method - that doesn't use any lookup mechanism,
and I wanted that there be no ambiguity that a new instance will be
returned with each invocation - I thought it might be better to
use "Creates" in all new methods - regardless of what other verb
the other method use - so for this feature, stay consistent across
factories instead of being consistent within the factory.
I can revert that if there's a general feeling that staying
consistent with the other methods in the same file is preferable.
- "Otherwise," and "Otherwise" (with and without ",") are not
consistent in the webrev.
Right - I will add the coma where it's missing. "Otherwise, " seems
to be more widely used.
XMLInputFactory.java:
-line 212: the "system-default" should be inside the braces with the
@link.
Good catch!
SchemaFactory.java:
- line 201: (pre-existing typo) "in a implementation" -> "in an
implementation"
OK.
XPathFactory.java
- line 177: The {@linkplain platform...} doesn't look like a properly
formed link.
Good catch again. How did I miss that?
XMLOutputFactoryNewInstanceTest
- line 2, has a 1999 date but is a new file.
Right. It's surprising that this file did not exits.
I copied XMLInputFactoryNewInstanceTest and subtituted
Input for Output...
Many of the files have long lines (some new) that make side-by-side
compares not fit on the screen.
I will crop the new and changed lines - but I don't want
to fix more as it would dilute the meaningful changes in noise.
Here is a new webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8169778/webrev.01/index.html
-- daniel
Regards, Roger
On 11/16/2016 10:24 AM, Daniel Fuchs wrote:
Hi,
Please find below a patch for:
8169778: Add new public methods to get new instances of the
JAXP factories builtin system-default implementations
bug: https://bugs.openjdk.java.net/browse/JDK-8169778
webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8169778/webrev.00/
best regards,
-- daniel
