-
Notifications
You must be signed in to change notification settings - Fork 6.1k
JDK-8263468: New page for "recent" new API #4209
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
Conversation
# Conflicts: # src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/stylesheet.css
👋 Welcome back hannesw! A progress list of the required criteria for merging this PR into |
Webrevs
|
make/Docs.gmk
Outdated
@@ -332,6 +332,9 @@ define SetupApiDocsGenerationBody | |||
$$(eval $$(call create_overview_file,$1)) | |||
$1_OPTIONS += -overview $$($1_OVERVIEW) | |||
|
|||
# Add summary pages for new/deprecated APIs in recent releases | |||
$1_OPTIONS += --since 12,13,14,15,16,17 --since-label "New API since JDK 11" |
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.
How do you expect this to change as the JDK version is bumped? If there is an expected pattern, then maybe we should try to generate the list instead, so we don't need to manually update twice a year.
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.
I don't have a solution for this problem. AFAIK there are a few things that have to be "bumped" manually between releases, and this would be yet another. Obviously not a great solution.
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.
Yes, there are a few things, but in the build itself, we are down to a single config file today, so I would really appreciate if this could be figured out. I can provide the implementation for generating this, but I need to understand what the expected pattern is. From what I can see, it looks like $(sequence N, M) where N is the last LTS+1 and M is current JDK version. Then the string has "JDK N" in it. M is already well defined, so the only new input here is N, which we could move to the version numbers config file (make/conf/version-numbers.conf). Something like DEFAULT_VERSION_DOCS_API_SINCE=11. There is some additional boilerplate needed, and the sequence macro of course, but does this sound reasonable to you?
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.
We actually have a sequence macro already, so generating the list can be done like this:
Or if what you have is 11 and 17:
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.
Thanks for the help and for providing all the pieces, Erik! I just added a new commit and I think I got everything working.
Mailing list message from Jonathan Gibbons on javadoc-dev: On 5/27/21 6:31 AM, Erik Joelsson wrote:
The ability to reduce the configuration down to a single number, like -- Jon |
1 similar comment
Mailing list message from Jonathan Gibbons on javadoc-dev: On 5/27/21 6:31 AM, Erik Joelsson wrote:
The ability to reduce the configuration down to a single number, like -- Jon |
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.
Build changes look great!
@hns 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 21 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
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.
Some minor suggestions for your consideration
doclet.Deprecated_In_Release=Deprecated in {0} | ||
doclet.Deprecated_Tabs_Intro=(The leftmost tab "Deprecated ..." indicates all the \ | ||
deprecated elements, regardless of the releases in which they were deprecated. \ | ||
Each of the righthand tabs "Deprecated in ..." indicates the elements deprecated \ |
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.
"Each of the righthand" doesn't read well. Would "Each of the other" be better?
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.
Changed to "Each of the other".
doclet.New_Elements_Added_In_Release=Added in {0} | ||
doclet.New_Label=New | ||
doclet.New_Tabs_Intro=(The leftmost tab "New ..." indicates all the new elements, \ | ||
regardless of the releases in which they were added. Each of the righthand \ |
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.
ditto
@@ -245,6 +263,9 @@ doclet.help.deprecated.body=\ | |||
doclet.help.preview.body=\ | |||
The {0} page lists all of the Preview APIs. \ | |||
Preview APIs may be removed in future implementations. | |||
doclet.help.new.body=\ | |||
The {0} page lists APIs that have been added in recent releases. \ | |||
The content of this page is based on information provided by Javadoc @since tags. |
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.
Either change to "JavaDoc" or (preferably?) just delete this word, or even the sentence.
Is there any interaction with -nosince
? Should there be?
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.
I removed the second sentence.
There is no interaction with -nosince
. I one point I thought -nosince
should disable this feature, but actually both features are just different ways of displaying the information stored in @since
tags that don't depend on each other.
assert SourceVersion.valueOf("RELEASE_" + versionNumber) == sourceVersion; | ||
return Integer.parseInt(versionNumber); | ||
public int getSourceVersionNumber() { | ||
return configuration.docEnv.getSourceVersion().ordinal(); |
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.
As a general comment, I believe Joe does not encourage use of ordinal
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.
I undid this change as it's not really part of the feature.
* Generate File to list all the new API elements with the | ||
* appropriate links. |
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.
(minor) not standard form of comment, but it's "only" an internal class, so could be fixed up later
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.
This comment and the comment below was copied and adapter from another SummaryListWriter subclass. I took the liberty of rewriting both comments.
/** | ||
* The class of the {@code body} element for the page listing any deprecated items. | ||
*/ | ||
deprecatedInReleasePage, | ||
|
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.
Note to self ... affects new "Output Generated ...." document
@@ -145,6 +145,9 @@ public static DocPath indexN(int n) { | |||
/** The name of the fie for preview elements. */ | |||
public static final DocPath PREVIEW_LIST = DocPath.create("preview-list.html"); | |||
|
|||
/** The name of the fie for new elements. */ |
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.
typo "fie"
@@ -145,6 +145,9 @@ public static DocPath indexN(int n) { | |||
/** The name of the fie for preview elements. */ |
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.
typo: "fie"
return (String) getDeprecatedElement(e, "since"); | ||
} | ||
|
||
// Returns the Deprecated annotation element value of the given element, or null. |
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.
Use /**...*/
*/ | ||
module mdl { | ||
exports pkg; | ||
} |
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.
final newline
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.
For all these "sample API" source files, they are OK, but another time, consider the use of possibly-custom builders, to generate these files dynamically. and to make for more concise reading.
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.
One future suggestion.
doclet.Deprecated_Tabs_Intro=(The leftmost tab "Deprecated ..." indicates all the \ | ||
deprecated elements, regardless of the releases in which they were deprecated. \ | ||
Each of the other tabs "Deprecated in ..." indicates the elements deprecated \ | ||
in a specific release.) |
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.
This is OK for now, but a noreg-doc follow-up might be to improve the punctuation in this text, possibly after consulting with docs folk.
It's the use of the quoted strings that seems weird, without any commas or parentheses too set them off. On the other hand, I realize we sometimes go for brevity in places like this.
/integrate |
@hns Since your change was applied there have been 24 commits pushed to the
Your commit was automatically rebased without conflicts. Pushed as commit dc6c96b. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
This adds a new kind of summary list for new API added in specific releases, and adds information to the deprecated API list about elements that were deprecated in the given releases.
The changes to the code are relatively minor thanks to the existing infrastructure for summary list pages, which was extended by adding the
getTableCaption
andaddTableTabs
methods toSummaryListWriter.java
in order to generate tabbed tables.One important area that needs to be reviewed is the addition of resources in
standard.properties
. A relatively big share of discussion and effort went into shaping the UI messages.The build system change adds options to generate API changes for all releases after JDK 11, with "New API since JDK 11" as page title for the new API page. I uploaded the generated documentation here:
http://cr.openjdk.java.net/~hannesw/8263468/api-pr.00/new-list.html
http://cr.openjdk.java.net/~hannesw/8263468/api-pr.00/deprecated-list.html
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/4209/head:pull/4209
$ git checkout pull/4209
Update a local copy of the PR:
$ git checkout pull/4209
$ git pull https://git.openjdk.java.net/jdk pull/4209/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 4209
View PR using the GUI difftool:
$ git pr show -t 4209
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/4209.diff