On Sun, 30 Apr 2023 07:40:22 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Joe Wang has updated the pull request with a new target base due to a merge >> or a rebase. The incremental webrev excludes the unrelated changes brought >> in by the merge/rebase. The pull request contains ten additional commits >> since the last revision: >> >> - Merge branch 'master' into JDK-8303530 >> - Update based on the review meeting on 4/5. Added (duplicated) definitions >> in jaxp.properties; Rewrote Property Precedence with samples; Other updates. >> - update javadoc as discussed in the review meeting; add a sample >> configuration file jaxp.properties; update impl and tests accordingly. >> - continue support stax.properties at the impl level, though dropped from >> the spec >> - change prefix from jdk to java >> - change prefix from jdk to java; rm zip file that accidentally checked in >> - update config file description; add a general scope and order section; >> move value definition for external properties to class description >> - clean up tests; fix copy&paste error. >> - 8303530: Add system property for custom JAXP configuration file > > src/java.xml/share/classes/module-info.java line 264: > >> 262: * </li> >> 263: * <li><p> >> 264: * Properties set using the corresponding System properties; > > Can you confirm that this means system properties set on the command line > with -Dkey=value or System.setProperty(key, value)? > > (I understand that the catalog feature "RESOLVE" property helps to explain it > but I'm concerned that "system property" is ambiguous throughout). Yes. Took it as granted "system property" is well-understood. If it can still be confusing, added a section "Properties and System Properties" to make it clearer what "system property" meant throughout the document. > src/java.xml/share/classes/module-info.java line 332: > >> 330: * @implNote >> 331: * >> 332: * <h2 id="ConfigurationFile">JAXP Configuration File</h2> > > I thin this means there are two anchors with the same name and two sections > in the module description with the same title. I understand this is SE vs. > JDK specific properties but this could easily go over the headers of someone > reading this. I think it would be better if "Implementation Specific Features > and Properties" were the first section of the implNote. It can start by > saying that the JAXP Configuration file (link to the description at the top) > can also be used for JDK/implementation specific properties. Removed. Incorporated with the sub-section "configuration file" within the "Implementation Specific Features and Properties" section. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/12985#discussion_r1182968075 PR Review Comment: https://git.openjdk.org/jdk/pull/12985#discussion_r1182968959
