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
8295962: Reference to State in Task.java is ambiguous when building with JDK 19 #933
8295962: Reference to State in Task.java is ambiguous when building with JDK 19 #933
Conversation
👋 Welcome back kcr! A progress list of the required criteria for merging this PR into |
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.
all instances in Task class
@kevinrushforth
|
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.
see 9 errors.
why do we not see these errors in github actions checks?
edit: pls disregard the question - forgot we are building with 18 in github.
Thanks for reporting this, I'll fix them. I think I know why I missed them when I did my test run. |
The reason I didn't catch these is that If I temporarily modify my build to use I'll still fix those tests to make it more future proof. |
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.
please update copyright year in modified tests :-)
Sure. I generally don't manually update the copyright date (we run a script for that), but since I already did it for |
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.
looks good in eclipse with jdk-19
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.
LGTM, tested on Mac.
@kevinrushforth 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 no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
/integrate |
Going to push as commit 4a2afb4. |
@kevinrushforth Pushed as commit 4a2afb4. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
This PR replaces all occurrences of
State
withWorker.State
injavafx.concurrent.Task
.The
javafx.concurrent.Task
class implementsjavafx.concurrent.Worker
and extendsjava.util.concurrent.FutureTask
, which in turn implementsjava.util.concurrent.Future
.Worker
has a nestedState
enum, which is used in the implementingTask
class without being qualified -- sinceTask
is aWorker
, we can just sayState
instead ofWorker.State
.JDK-8277090 added a nested
State
enum to thejava.util.concurrent.Future
interface in JDK 19, so an unqualified reference toState
from theTask
class is now ambiguous when using JDK 19 to build. The javadoc task fails with an error (and the only reason the javac task doesn't is that we use--release 17
).With this fix, a local build and test using JDK 19 passes.
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx pull/933/head:pull/933
$ git checkout pull/933
Update a local copy of the PR:
$ git checkout pull/933
$ git pull https://git.openjdk.org/jfx pull/933/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 933
View PR using the GUI difftool:
$ git pr show -t 933
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/933.diff