Skip to content
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

8264137: Suppress deprecation and removal warnings of internal methods #538

Closed
wants to merge 4 commits into from

Conversation

@aghaisas
Copy link
Collaborator

@aghaisas aghaisas commented Jun 21, 2021

This PR suppresses -PLINT="removal" warnings and fixes -PLINT="deprecation" warnings. I have created separate commits for each type of warnings for the ease of review.

gradle --info -PLINT="deprecation,removal" -- results in warnings during build without this PR and results in no warnings with this PR.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issues

  • JDK-8264137: Suppress deprecation and removal warnings of internal methods
  • JDK-8252820: Skins: cleanup use of deprecated snapSize/snapPosition

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jfx pull/538/head:pull/538
$ git checkout pull/538

Update a local copy of the PR:
$ git checkout pull/538
$ git pull https://git.openjdk.java.net/jfx pull/538/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 538

View PR using the GUI difftool:
$ git pr show -t 538

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jfx/pull/538.diff

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Jun 21, 2021

👋 Welcome back aghaisas! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr label Jun 21, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Jun 21, 2021

Webrevs

@kevinrushforth kevinrushforth self-requested a review Jun 21, 2021
@kevinrushforth
Copy link
Member

@kevinrushforth kevinrushforth commented Jun 21, 2021

I tested this patch, and verified that it does eliminate the deprecation and removal warnings when building the sdk.

There are still a few warnings when compiling the tests. I ran the following command:

$ gradle --continue --info -PLINT="deprecation,removal" test

I get the following deprecation warnings:

modules\javafx.graphics\src\shims\java\javafx\scene\input\GestureEventShim.java:48: warning: [deprecation] GestureEvent(Object,EventTarget,EventType<? extends GestureEvent>) in GestureEvent has been deprecated
modules\javafx.graphics\src\test\java\test\javafx\css\StylesheetTest.java:628: warning: [deprecation] toURL() in File has been deprecated
modules\javafx.graphics\src\test\java\test\javafx\scene\Scenegraph_eventHandlers_Test.java:342: warning: [deprecation] EventType() in EventType has been deprecated
modules\javafx.controls\src\test\java\test\javafx\scene\chart\XYChartTest.java:92: warning: [deprecation] set(S,V,StyleOrigin) in CssMetaData has been deprecated
modules\javafx.controls\src\test\java\test\javafx\scene\control\LabeledTest.java:768: warning: [deprecation] set(S,V,StyleOrigin) in CssMetaData has been deprecated
modules\javafx.controls\src\test\java\test\javafx\scene\control\TreeItemTest.java:201: warning: [deprecation] getNodeLevel(TreeItem<?>) in TreeView has been deprecated
modules\javafx.controls\src\test\java\test\javafx\scene\control\TreeItemTest.java:208: warning: [deprecation] getNodeLevel(TreeItem<?>) in TreeView has been deprecated
modules\javafx.controls\src\test\java\test\javafx\scene\control\TreeItemTest.java:217: warning: [deprecation] getNodeLevel(TreeItem<?>) in TreeView has been deprecated
modules\javafx.controls\src\test\java\test\javafx\scene\control\TreeItemTest.java:231: warning: [deprecation] getNodeLevel(TreeItem<?>) in TreeView has been deprecated
modules\javafx.controls\src\test\java\test\javafx\scene\control\TreeItemTest.java:232: warning: [deprecation] getNodeLevel(TreeItem<?>) in TreeView has been deprecated
modules\javafx.fxml\src\test\java\test\com\oracle\javafx\fxml\test\TestLoadPerformance.java:107: warning: [deprecation] XMLReaderFactory in org.xml.sax.helpers has been deprecated
tests\system\src\test\java\test\robot\com\sun\glass\ui\monocle\input\devices\TestTouchDevices.java:46: warning: [deprecation] newInstance() in Class has been deprecated
tests\system\src\test\java\test\robot\javafx\embed\swing\JFXPanelTest.java:95: warning: [deprecation] BUTTON1_MASK in InputEvent has been deprecated
tests\system\src\test\java\test\robot\javafx\embed\swing\JFXPanelTest.java:96: warning: [deprecation] BUTTON1_MASK in InputEvent has been deprecated
tests\system\src\test\java\test\robot\javafx\embed\swing\NonFocusableJFXPanelTest.java:74: warning: [deprecation] BUTTON1_MASK in InputEvent has been deprecated
tests\system\src\test\java\test\robot\javafx\embed\swing\NonFocusableJFXPanelTest.java:75: warning: [deprecation] BUTTON1_MASK in InputEvent has been deprecated

and the following removal warnings:

modules\javafx.base\src\shims\java\javafx\util\converter\DateTimeStringConverterShim.java:34: warning: [removal] timeStyle in DateTimeStringConverter has been deprecated and marked for removal
modules\javafx.base\src\shims\java\javafx\util\converter\DateTimeStringConverterShim.java:38: warning: [removal] pattern in DateTimeStringConverter has been deprecated and marked for removal
modules\javafx.base\src\shims\java\javafx\util\converter\DateTimeStringConverterShim.java:42: warning: [removal] dateStyle in DateTimeStringConverter has been deprecated and marked for removal
modules\javafx.base\src\shims\java\javafx\util\converter\DateTimeStringConverterShim.java:46: warning: [removal] getDateFormat() in DateTimeStringConverter has been deprecated and marked for removal
modules\javafx.base\src\shims\java\javafx\util\converter\DateTimeStringConverterShim.java:50: warning: [removal] dateFormat in DateTimeStringConverter has been deprecated and marked for removal
modules\javafx.base\src\shims\java\javafx\util\converter\DateTimeStringConverterShim.java:54: warning: [removal] locale in DateTimeStringConverter has been deprecated and marked for removal

You can either fix it as part of this bug fix or else file a follow-on bug for the tests.

@kevinrushforth
Copy link
Member

@kevinrushforth kevinrushforth commented Jun 21, 2021

I'd like a second pair of eyes on this. The changes are very straight-forward, but there are a lot of them and it would be easy to have a copy-paste error and use snapSizeX instead of snapSizeY or vice versa.

/reviewers 2

@kevinrushforth kevinrushforth requested a review from arapte Jun 21, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Jun 21, 2021

@kevinrushforth
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

Copy link
Member

@kevinrushforth kevinrushforth left a comment

Looks good.

Copy link
Member

@Maran23 Maran23 left a comment

Looks good. I double checked the snapSizeX/Y and snapPositionX/Y methods, I didn't found any location where the wrong method is used.
Good to see that all deprecated snapSize/snapPosition usages are finally replaced. :)

Btw, it looks like this PR also resolves: https://bugs.openjdk.java.net/browse/JDK-8252820
So this ticket can be either added to the issues this PR resolves or be closed.

@openjdk
Copy link

@openjdk openjdk bot commented Jun 22, 2021

@aghaisas 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:

8264137: Suppress deprecation and removal warnings of internal methods
8252820: Skins: cleanup use of deprecated snapSize/snapPosition

Reviewed-by: kcr, mhanl

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 3 new commits pushed to the master branch:

  • 8e11b94: 8268915: WebKit build fails with Xcode 12.5
  • 45786ac: 8252238: TableView: Editable (pseudo-editable) cells should respect the row editability
  • 04493e5: 8165214: ListView.EditEvent.getIndex() does not return the correct index

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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 master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready label Jun 22, 2021
@aghaisas
Copy link
Collaborator Author

@aghaisas aghaisas commented Jun 23, 2021

You can either fix it as part of this bug fix or else file a follow-on bug for the tests.

Thanks for reporting these warnings. I will fix them as part of the same PR.

@openjdk openjdk bot removed the ready label Jun 23, 2021
@aghaisas
Copy link
Collaborator Author

@aghaisas aghaisas commented Jun 23, 2021

/issue add JDK-8252820

@openjdk
Copy link

@openjdk openjdk bot commented Jun 23, 2021

@aghaisas
Adding additional issue to issue list: 8252820: Skins: cleanup use of deprecated snapSize/snapPosition.

Copy link
Member

@kevinrushforth kevinrushforth left a comment

Test changes look good, too.

Copy link
Member

@Maran23 Maran23 left a comment

Looks good. 👍

@openjdk openjdk bot added the ready label Jun 23, 2021
@aghaisas
Copy link
Collaborator Author

@aghaisas aghaisas commented Jun 23, 2021

/integrate

@openjdk
Copy link

@openjdk openjdk bot commented Jun 23, 2021

Going to push as commit 063bfe8.
Since your change was applied there have been 3 commits pushed to the master branch:

  • 8e11b94: 8268915: WebKit build fails with Xcode 12.5
  • 45786ac: 8252238: TableView: Editable (pseudo-editable) cells should respect the row editability
  • 04493e5: 8165214: ListView.EditEvent.getIndex() does not return the correct index

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot closed this Jun 23, 2021
@openjdk openjdk bot added integrated and removed ready rfr labels Jun 23, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Jun 23, 2021

@aghaisas Pushed as commit 063bfe8.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@aghaisas aghaisas deleted the warning_fix branch Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants