-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8370154: Update @jls and @jvms taglets to point to local specs dir #28207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
👋 Welcome back dlsmith! A progress list of the required criteria for merging this PR into |
|
@dansmithcode This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 2 new commits pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
|
@dansmithcode The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
|
Looks goood. Meanwhile, do you think we should retain external link support via a system property? For the Java SE API specification only distribution for JCP, we might want to link to a static link instead of a local relative link. (I think as long as we can link, a relative link like here is better because it will be stable in the specdiff for JCP review, avoiding spams) Since we removed the system property jspec.version, we should be able to remove this from the build files too: Line 295 in 066810c
|
|
JCP reviews actually happen before GA, so I think the external links are always dead anyway? If it's something the JCP folks care about, I think the right solution is to include JLS & JVMS in the SE-only docs. (They are, after all, specs for Java SE.) But for now, I think the business as usual process is fine. |
|
I'll confirm there are no more uses of |
|
I think we can go ahead with this once we verify Oracle's doc build passes. I will approve once the system property passing in Docs.gmk is updated. |
|
I tested this patch on my local Linux-targeting OpenJDK WSL (because only Linux is building docs); the JVMS links become dead links in OpenJDK builds, which don't have access to JLS/JVMS, which are proprietary to Oracle. We should probably leave a comment that the dead links are intentional. |
liach
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified this passes on Oracle's CI.
Updated the JSpec taglets to use relative links into
../specs/jlsand../specs/jvmsrather than external links, possible now due to JDK-8370153.I was surprised there doesn't seem to be a standard way to get a relative path to the docs root, but I copy/pasted from ToolGuide, and made some tweaks based on failures I saw.
Tested with 'make docs' and manually confirmed that a handful of different
@jlsand@jvmstags are pointing to the right place.Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28207/head:pull/28207$ git checkout pull/28207Update a local copy of the PR:
$ git checkout pull/28207$ git pull https://git.openjdk.org/jdk.git pull/28207/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 28207View PR using the GUI difftool:
$ git pr show -t 28207Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28207.diff
Using Webrev
Link to Webrev Comment