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
8332103: Add missing @since
tags to java.desktop
#19192
base: master
Are you sure you want to change the base?
Conversation
👋 Welcome back nizarbenalla! A progress list of the required criteria for merging this PR into |
❗ This change is not yet ready to be integrated. |
@nizarbenalla The following label 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 list. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
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 did verify the updates against the release versions and looks good to me.
@@ -149,5 +149,7 @@ | |||
* </ul> | |||
* | |||
* @serial exclude | |||
* | |||
* @since 1.1 |
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 isn't right. Where did you get this from ?
Swing only became part of the JDK in JDK 1.2
It was an unbundled library before then.
If you find any @SInCE 1.1 tags in the Swing API they are a mistake.
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.
My bad then, this is a mistake.
@@ -150,6 +150,8 @@ public class BasicSliderUI extends SliderUI{ | |||
|
|||
/** | |||
* Constructs a {@code BasicSliderUI}. | |||
* | |||
* @since 16 |
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.
Hmm, the explicit default constructor was added in JDK 16, but it was implicit before then.
So I am not 100% sure what the right answer is - the same as the class, or when it was explicitly added.
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.
When mapping methods and when they first appeared (by using the historical record built into javac) I use an id in the form of
method: <erased-return-descriptor> <binary-name-of-enclosing-class>.<method-name>(<ParameterDescriptor>)
so for covariant overrides in general, when the return type changes I consider it to be a new method.
Looking at the contents of the dictionnary:
This explicit constructor existed for a long time but then this new was added a new one was added in JDK 16
Key | Value |
---|---|
method: void javax.swing.plaf.basic.BasicSliderUI.<init>(javax.swing.JSlider): |
9 |
method: void javax.swing.plaf.basic.BasicSliderUI.<init>(): |
16 |
Note: JDK 9 is used as the "base" as that's how far I can reliably use the --release
info, so if something was added in JDK 2,5.7,9. It has a value of "9" in the dictionnary. I mainly check for errors in newer code.
I'm not sure I understand the methodology here. |
As I'm using the historical data built into
but for older code you can only guess "Element: X existed before JDK 10". So I was left to check on my own, and made a mistake. |
If you're currently reviewing this PR, thank you!
Most fixes here are according to the reports by the since checker tool in #18934 and are pretty simple.
To make reviewing easier
BasicSliderUI
has the constructorpublic BasicSliderUI(JSlider b)
for a long time so the default constructor (without parameters) didn't exist until JDK 16For the
package-info
files, it is pretty hard to find source code of JDK 1-5 so I used thegrep
command to find the oldest instance of an@since
in those packages.I found instances of
@since 1.1
in the other packages butjavax/swing/plaf/synth/package-info.java
might be worth checking as most classes there had no@since
.Progress
Issue
@<!---->since
tags tojava.desktop
(Sub-task - P4)Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19192/head:pull/19192
$ git checkout pull/19192
Update a local copy of the PR:
$ git checkout pull/19192
$ git pull https://git.openjdk.org/jdk.git pull/19192/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 19192
View PR using the GUI difftool:
$ git pr show -t 19192
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19192.diff
Webrev
Link to Webrev Comment