8294872: Document "resource" protocol - #2251
Conversation
|
👋 Welcome back almatvee! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
The total number of required reviews for this PR has been set to 2 based on the presence of this label: |
Webrevs
|
| * <li>The supplied URI must conform to RFC-2396 as required by | ||
| * <A href="https://docs.oracle.com/javase/8/docs/api/java/net/URI.html">java.net.URI</A>.</li> | ||
| * <li>Only HTTP, HTTPS, FILE, and JAR URIs are supported.</li> | ||
| * <a href="https://docs.oracle.com/javase/8/docs/api/java/net/URI.html">java.net.URI</a>.</li> |
There was a problem hiding this comment.
why are we referencing java8 docs?
shouldn't it be a relative ref?
There was a problem hiding this comment.
URI is not part of JavaFX, so we cannot use relative ref. Also, I found that JavaFX docs a published in several places, so using something like "../../URI.html" will not work.
I can update Java 8 links to latest, but I am not sure what policy we have in term of referencing old JDK releases and should we keep updating it to latest docs with each release. For example JavaFX 27 will point to JDK 27 docs.
There was a problem hiding this comment.
Gradle is configured to generate Javadocs with links to the documentation for the supported JDK version.
See:
Therefore, you should be able to replace:
<a href="https://docs.oracle.com/javase/8/docs/api/java/net/URI.html">java.net.URI</a>
with:
{@link java.net.URI}
| * for more information about URI formatting in general. | ||
| * JAR URL syntax is specified in <a href="https://docs.oracle.com/javase/8/docs/api/java/net/JarURLConnection.html">java.net.JarURLConnection</A>. | ||
| * JAR URL syntax is specified in <a href="https://docs.oracle.com/javase/8/docs/api/java/net/JarURLConnection.html">java.net.JarURLConnection</a>. | ||
| * RESOURCE URL syntax is specified in <a href="https://www.graalvm.org/latest/reference-manual/native-image/dynamic-features/URLProtocols/">URL Protocols in Native Image</a>. |
There was a problem hiding this comment.
similar question: why refer to graalvm.org?
shouldn't we point to some authoritative document instead?
There was a problem hiding this comment.
I found this one https://docs.oracle.com/en/graalvm/jdk/25/docs/reference-manual/native-image/dynamic-features/URLProtocols/, but it does not point to latest as graalvm.org. I do not see any problems with pointing to graalvm.org. Is there any concerns?
| * Constructs a <code>Media</code> instance. This is the only way to | ||
| * specify the media source. The source must represent a valid <code>URI</code> | ||
| * and is immutable. Only HTTP, HTTPS, FILE, and JAR <code>URL</code>s are supported. If the | ||
| * and is immutable. Only HTTP, HTTPS, FILE, JAR, and RESOURCE <code>URL</code>s are supported. If the |
There was a problem hiding this comment.
just out of curiosity, could we use the actual codes ("https:", "jar:", ...) instead of HTTPS, JAR, ...?
|
8294872: Document "resource" protocol
|
|
/issue add JDK-8374836 |
|
@sashamatveev |
| * @param source URL string from which to load the audio clip. This can be an | ||
| * HTTP, HTTPS, FILE or JAR source. | ||
| * <code>http</code>, <code>https</code>, <code>file</code>, <code>jar</code>, | ||
| * <code>jrt</code>, and <code>resource</code> source. |
There was a problem hiding this comment.
"resource source" is poorly phrased. suggestion:
Supported schemes are ....
| * | ||
| * <table border="1"> | ||
| * <caption>Supported Protocols Table</caption> | ||
| * <caption>Supported Schemes / Protocols Table</caption> |
There was a problem hiding this comment.
L119: the header says "Protocol" while the table lists schemes and protocols.
I understand there is some mixing of related things - for example, the "P" in HTTP is for "protocol", but "http" (or "http:") is a URI scheme.
So maybe revert adding "Schemes /" in L115,118 and just keep saying "protocol"?
What do you think?
Progress
Issues
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/2251/head:pull/2251$ git checkout pull/2251Update a local copy of the PR:
$ git checkout pull/2251$ git pull https://git.openjdk.org/jfx.git pull/2251/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 2251View PR using the GUI difftool:
$ git pr show -t 2251Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/2251.diff
Using Webrev
Link to Webrev Comment