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

8207957: TableSkinUtils should not contain actual code implementation #6

Closed
wants to merge 14 commits into from

Conversation

@Maxoudela
Copy link

Maxoudela commented Oct 3, 2019

Allright, this is a fix for JDK-8207957

Progress

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

Issue

JDK-8207957: TableSkinUtils should not contain actual code implementation

Approvers

  • Jeanette Winzenburg (fastegal - Author) Note! Review applies to e1a9d2d
  • Kevin Rushforth (kcr - Reviewer)
  • Ajit Ghaisas (aghaisas - Reviewer)
@bridgekeeper bridgekeeper bot added the oca label Oct 3, 2019
@bridgekeeper
Copy link

bridgekeeper bot commented Oct 3, 2019

Hi Maxoudela, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user Maxoudela" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

@bridgekeeper bridgekeeper bot removed the oca label Oct 4, 2019
@openjdk openjdk bot added the rfr label Oct 4, 2019
@mlbridge
Copy link

mlbridge bot commented Oct 4, 2019

@kevinrushforth
Copy link
Member

kevinrushforth commented Oct 4, 2019

JBS bug: JDK-8207957

Review comments from preliminary review: javafxports/openjdk-jfx/pull/289

protected void resizeColumnToFitContent(TableColumnBase<?, ?> tc, int maxRows) {
if (!tc.isResizable()) return;
Comment on lines 606 to 607

This comment has been minimized.

Copy link
@kleopatra

kleopatra Oct 7, 2019

Contributor

see javafxports/openjdk-jfx#289 (comment) - this api still looks unhealthy ..

* Resizes the given column based on the preferred width of all items contained in it. This can be potentially very
* expensive if the number of rows is large. Subclass can either call this method or override it (no need to call
Comment on lines 598 to 599

This comment has been minimized.

Copy link
@kleopatra

kleopatra Oct 7, 2019

Contributor

see javafxports/openjdk-jfx#289 (comment) - I think I made a suggestion (that probably needs some native speaker's fine tuning :)

…ation on TableColumnHeader
@Maxoudela
Copy link
Author

Maxoudela commented Oct 9, 2019

Allright @kleopatra , I have indeed removed the TableColumn argument. It is clearer now that we are resizing the TableColumn of the header.

I have updated the description a bit but really I'm really not good at method description so I'm open to all suggestions..

Copy link
Contributor

nlisker left a comment

Since you are adding new API you will need to file a CSR once the API review id done.

TableSkinUtils.resizeColumnToFitContent(header.getTableSkin(), column, -1);
TableHeaderRow tableHeader = header.getTableHeaderRow();
TableColumnHeader columnHeader = tableHeader.getColumnHeaderFor(column);
if(columnHeader != null){

This comment has been minimized.

Copy link
@nlisker

nlisker Oct 9, 2019

Contributor

Space after if and before {.

* Resizes this {@code TableColumnHeader}'s column based on content of header and content of cells. This
* implementation measures the preferred width of the header, the preferred width of the first {@code maxRow} cells
* and sizes the column to the maximum width of all measured values. Subclass can either call this method or
* override it (no need to call {@code super()}) to provide their custom implementation (exclude headers, exclude
* null content, use min).
*
* @param maxRows the number of rows considered when resizing. If -1 is given, all rows are considered.
* @since 14
Comment on lines 598 to 605

This comment has been minimized.

Copy link
@nlisker

nlisker Oct 9, 2019

Contributor

Here's my suggestion (mind the max character length per line):

Resizes this {@code TableColumnHeader}'s column to fit the width of its content. The resulting column width is the maximum of the preferred width of the header cell and the preferred width of the first {@code maxRow} cells.
<p>
Subclasses can either use this method or override it (without the need to call {@code super()}) to provide their custom implementation (such as ones that exclude the header, exclude {@code null} content, compute the minimum width etc.).

This comment has been minimized.

Copy link
@Maxoudela

Maxoudela Oct 10, 2019

Author

Seems good but I have a minor doubt on "header cell". Can the header be considered as a "cell"?

EDIT: I have pushed the modifications without the "cell", let me know what you think of it.

This comment has been minimized.

Copy link
@nlisker

nlisker Oct 10, 2019

Contributor

I don't mind either way, but I think that you're right, the header is not technically a cell.

This comment has been minimized.

Copy link
@kleopatra

kleopatra Oct 10, 2019

Contributor

Allright @kleopatra , I have indeed removed the TableColumn argument. It is clearer now that we are resizing the TableColumn of the header.

good, IMO :) Are there any tests guaranteeing that new behaviour is the same as old behaviour?

This comment has been minimized.

Copy link
@kleopatra

kleopatra Oct 10, 2019

Contributor

Resizes this {@code TableColumnHeader}'s column to fit the width of its content. The resulting column width is the maximum of the preferred width of the header cell and the preferred width of the first {@code maxRow} cells.

Subclasses can either use this method or override it (without the need to call {@code super()}) to provide their custom implementation (such as ones that exclude the header, exclude {@code null} content, compute the minimum width etc.). ```

still would prefer to emphasize a bit more that the sizing you describe in the second sentence is just some implementation (could be implied from the second paragraph, though). How about:

The resulting column width for this implementation is the ...

This comment has been minimized.

Copy link
@Maxoudela

Maxoudela Oct 11, 2019

Author

I wouldn't know if there are enough tests on that part. I know I added one.

resizeColumnToFitContent((TableView)control, (TableColumn)tc, this.getTableSkin(), maxRows);
} else if (control instanceof TreeTableView) {
resizeColumnToFitContent((TreeTableView)control, (TreeTableColumn)tc, this.getTableSkin(), maxRows);
Comment on lines 613 to 615

This comment has been minimized.

Copy link
@nlisker

nlisker Oct 9, 2019

Contributor

I think we put a space when casting, like (TableView) control. Not sure if it is a rule.

@kevinrushforth
Copy link
Member

kevinrushforth commented Oct 9, 2019

A Draft CSR was filed here: JDK-8215554. It will need to be updated once the API review is complete.

Samir Hadzic added 2 commits Oct 10, 2019
@Maxoudela
Copy link
Author

Maxoudela commented Oct 29, 2019

Do you think this looks good now @kleopatra @nlisker ?

@Maxoudela
Copy link
Author

Maxoudela commented Nov 1, 2019

@kleopatra
Copy link
Contributor

kleopatra commented Nov 7, 2019

As to the test, it's a bit ... confusing, will try to suggest some changes - take them with a grain of salt, though, because I'm still groping to find my path through the wilderness here :)

Copy link
Contributor

kleopatra left a comment

An open question for me is: what is the standard procedure here?

a) there are no tests around sizing (that I could find, maybe overlooked them)
b) the change in this pull request is (more or less) simply moving around old code and pulling from private into protected scope, no changes in implementation/functionality

Now is the contributor responsible for writing those missing tests? It can be considered a good opportunity ;) And strictly speaking is mandatory (probably?) for all public/protected api. On the other extreme, doing nothing (no tests because basically nothing changed) might be appropriate as well.

private MyTableColumnHeader tableColumnHeader;

Comment on lines 53 to 54

This comment has been minimized.

Copy link
@kleopatra

kleopatra Nov 7, 2019

Contributor

If I understand the intention correctly, the only reason for subclassing the TableView with a custom skin, headerRow, etc deep down into a custom columnHeader is to access its protected resizeColumnToFitContent? If so, an alternative approach might be to

a) add a accessing method to TableColumnHeaderUtil in the shims hierarchy
b) don't subclass but use that new accessor

This comment has been minimized.

Copy link
@Maxoudela

Maxoudela Nov 7, 2019

Author

But should I be able to call the protected method? I'll take a look to see what I can do

This comment has been minimized.

Copy link
@kleopatra

kleopatra Nov 7, 2019

Contributor

yes, the shim and/or the utiliy class can (they reside in the xx.skin package, so have access to protected/package-private stuff

@Test
public void test_resizeColumnToFitContent() {
Comment on lines 60 to 61

This comment has been minimized.

Copy link
@kleopatra

kleopatra Nov 7, 2019

Contributor

This method actually is three-in-one :) It's testing

a) trigger a resize initially doesn't change the with
b) change content to larger and trigger a resize increases column width
c) change content to smaller and trigger a resize decreases column width

Not sure about conventions here, but I would separate them out into distinct methods.

This comment has been minimized.

Copy link
@Maxoudela

Maxoudela Nov 7, 2019

Author

Well that depends on the point of view. From my point of view, I'm testing the "resize to fit content" feature. So I'm testing one feature and therefore the same size, a bit smaller and a bit greater.
But we could split this into 3 separate tests if that is the convention.

This comment has been minimized.

Copy link
@kleopatra

kleopatra Nov 7, 2019

Contributor

so you end up with one test per public/protected method? Hmm ..

EventType<TableColumn.CellEditEvent<Person, String>> eventType = TableColumn.editCommitEvent();
column.getOnEditCommit().handle(new TableColumn.CellEditEvent<Person, String>(
tableView, new TablePosition<Person, String>(tableView, 0, column), (EventType) eventType, "This is a big text inside that column"));
Comment on lines 115 to 117

This comment has been minimized.

Copy link
@kleopatra

kleopatra Nov 7, 2019

Contributor

that looks like a rather convoluted way of changing content - what's wrong with straightforward changing the firstName of the first item :)

This comment has been minimized.

Copy link
@Maxoudela

Maxoudela Nov 7, 2019

Author

Changing the firstName of the firstItem would be another test IMHO. It would test the fact that the backing model has changed and needs to be reflected in the view.

Here I want to test the fact that a user has modified a cell through the view, and it impacts the model.

This comment has been minimized.

Copy link
@kleopatra

kleopatra Nov 7, 2019

Contributor

hmm ... tend to disagree: I see the resize feature/api as independent on how the data that is measured is changed. There's no automatic resize on editing (nor on changing content), you manually trigger the method, so keep the test as simple as possible.

This comment has been minimized.

Copy link
@Maxoudela

Maxoudela Nov 7, 2019

Author

Allright, I can do it like that if you prefer

This comment has been minimized.

Copy link
@kleopatra

kleopatra Nov 7, 2019

Contributor

ehh ... do what you want - I have no rights whatsoever to force (not even nudge :) you into anything. Just been there and thought you might want a helping hand.

tableColumnHeader.resizeCol();
assertTrue("Column width must be smaller",
width > column.getWidth());
Comment on lines 131 to 133

This comment has been minimized.

Copy link
@kleopatra

kleopatra Nov 7, 2019

Contributor

same as above but the other way round.

Until, the tests verified that the column width is effected by cell content and goes into the course direction of what is expected. What's still missing are tests that cover the complete specification (we now have one :), that is verify that/how resize is indeed depend on

  • header content
  • maxRows

Plus maybe that custom implementations are respected: f.i. doing nothing, making them constant, header only

@kevinrushforth
Copy link
Member

kevinrushforth commented Nov 15, 2019

looks good to me - would approve if I could :)

You can add yourself as a reviewer, even though it wouldn't count as an approving review by someone with a "R"eviewer role in the project.

Copy link
Contributor

kleopatra left a comment

looks okay to me :)

Copy link
Member

kevinrushforth left a comment

The fix looks good to me.

I left a few minor comments, including adding a missing comma in the API docs, which will also need to be changed in the CSR. Once this is updated I'll review the CSR and then you can move the CSR to Finalize.

@kevinrushforth
Copy link
Member

kevinrushforth commented Dec 9, 2019

I would like both @aghaisas and myself to review / approve this. I will sponsor it.

In addition, the CSR needs to be approved before this can be integrated.

@kevinrushforth kevinrushforth self-requested a review Dec 9, 2019
@Maxoudela
Copy link
Author

Maxoudela commented Dec 9, 2019

@kevinrushforth
Copy link
Member

kevinrushforth commented Dec 11, 2019

I took the liberty of updating the CSR to add the missing comma, so I could mark it as Reviewed. You can Finalize the CSR when you are ready.

Samir Hadzic
@openjdk openjdk bot removed the rfr label Dec 16, 2019
Samir Hadzic
@openjdk openjdk bot added the rfr label Dec 16, 2019
@Maxoudela
Copy link
Author

Maxoudela commented Dec 16, 2019

I took the liberty of updating the CSR to add the missing comma, so I could mark it as Reviewed. You can Finalize the CSR when you are ready.

Thank you, I have committed new changes according to your review. I have also moved the CSR to "finalize".

Let me know if I need to do something else, sorry for the delay.

@openjdk openjdk bot removed the rfr label Dec 16, 2019
@openjdk
Copy link

openjdk bot commented Dec 16, 2019

@Maxoudela This change can now be integrated. The commit message will be:

8207957: TableSkinUtils should not contain actual code implementation

Reviewed-by: fastegal, kcr, aghaisas
  • If you would like to add a summary, use the /summary command.
  • To list additional contributors, use the /contributor command.

Since the source branch of this PR was last updated there have been 61 commits pushed to the master branch:

  • 935e99d: 8207957: TableSkinUtils should not contain actual code implementation
  • 4e005e4: 8227808: Make GTK3 libraries mandatory for building on Linux
  • 1140d34: 8196587: Remove use of deprecated finalize method from JPEGImageLoader
  • d2d44b4: 8207759: VK_ENTER not consumed by TextField when default Button exists
  • fc539b5: 8223296: NullPointerException in GlassScene.java at line 325
  • 71ca899: 8220722: ProgressBarSkin: adds strong listener to control's width property
  • 07af89a: 8221334: TableViewSkin: must initialize flow's cellCount in constructor
  • 4ddf554: 8234916: [macos 10.15] Garbled text running with native-image
  • 46338d0: 8232524: SynchronizedObservableMap cannot be be protected for copying/iterating
  • a68347c: 8235150: IosApplication does not pass the required object in _leaveNestedEventLoopImpl
  • 1c27fbd: 8210955: DOMTest::testEventListenerCascade fails
  • 2d4096a: 8235151: Nonexistent notifyQuit method referred from iOS GlassHelper.m
  • 98035cb: 8211308: Support HTTP/2 in WebView
  • 6892fa1: 8232064: Switch FX build to use JDK 13.0.1 as boot JDK
  • 1d670f1: 8200224: Multiple press event when JFXPanel gains focus
  • 83eb0a7: 8193445: JavaFX CSS is applied redundantly leading to significant performance degradation
  • 798afbc: 8230610: Upgrade GStreamer to version 1.16.1
  • 126896d: 8234704: Fix attribution in libxslt.md
  • 4d3c723: 8234593: Mark LeakTest.testGarbageCollectability as unstable
  • 5a39824: 8234056: Upgrade to libxslt 1.1.34
  • 8bea7b7: 8229472: Deprecate for removal JavaBeanXxxPropertyBuilders constructors
  • aad1720: 8233420: Upgrade to gcc 8.3 on Linux
  • 42040c4: 8232063: Upgrade gradle to version 6.0
  • aab07a4: 8234239: [TEST_BUG] Reenable few ignored web tests
  • 95ad601: 8233421: Upgrade to Visual Studio 2017 version 15.9.16
  • 3e0557a: 8234303: [TEST_BUG] Correct ignore tag in graphics unit tests
  • e37cb37: 8234150: Address ignored tests in ComboBoxTest, LabeledTest, HyperLinkTest and TextInputControlTest
  • 4f496d4: 8234194: [TEST_BUG] Reenable few graphics unit tests
  • 927fc8a: 8234174: Change IDEA VCS mapping to Git
  • 3d0cb49: 8234189: [TEST_BUG] Remove ignored and invalid graphics unit tests
  • dc01309: 8234110: SwingFXUtilsTest is unsuitable for unit test framework
  • 5b96ee4: 8231188: Update SQLite to version 3.30.1
  • d46dcae: 8233338: FX javadoc headings are out of sequence
  • 94bcf3f: 8231692: Test Infrastructure: enhance KeyEventFirer to inject keyEvents into scene
  • 286d1b5: 8230492: font-family not set in HTMLEditor if font name has a number in it
  • f74f3af: 8233040: ComboBoxPopupControl: remove eventFilter for F4
  • a1cc4ab: 8232210: Update Mesa 3-D Headers to version 19.2.1
  • dca8df4: 8232943: Gesture support is not initialized on iOS
  • 5a70b0c: 8189092: ArrayIndexOutOfBoundsException on Linux in getCachedGlyph
  • ac71396: 8232929: Duplicate symbols when building static libraries
  • ab6ea3b: 8232158: [macOS] Fallback to command line tools if xcode is missing
  • 2ae171a: 8232687: No static JNI loader for libprism-sw
  • a09a0fa: 8232522: FX: Update copyright year in docs, readme files to 2020
  • 582d999: Merge
  • b6e53f4: 8218640: Update ICU4C to version 64.2
  • bada612: Merge
  • 64aaeb8: 8226754: FX build fails using gradle 5.6+ or 6
  • 63fe66c: 8231870: CrossLibs script for armv6hf toolchain download fails
  • a4bc22d: Merge
  • c6eb091: 8231854: Change Mercurial to git in various README files
  • 0a0d34a: 8231590: Update location of jfx repo to GitHub in third-party legal files
  • f595cc1: 8231735: gradle checkrepo is obsolete and doesn't work with git
  • 2593dea: Merge
  • 9df20c5: Merge
  • a1aa38a: Merge
  • 0ff02bb: Merge
  • a433bf2: Merge
  • 561153d: Merge
  • 75d439e: Merge
  • 758252f: Merge
  • 35e0cae: 8227402: Improve XSLT processing

Since there are no conflicts, your changes will automatically be rebased on top of the above commits when integrating. If you prefer to do this manually, please merge master into your branch first.

As you do not have Committer status in this project, an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@nlisker, @kevinrushforth, @aghaisas) but any other Committer may sponsor as well.

  • To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).
@openjdk openjdk bot added the ready label Dec 16, 2019
@kleopatra
Copy link
Contributor

kleopatra commented Dec 16, 2019

hmm ... think that the bot isn't yet clever enough: this pull request needs approval of two reviewer with review role (mine is only informal)

@kevinrushforth
Copy link
Member

kevinrushforth commented Dec 16, 2019

This one still needs an approved CSR (the CSR is Finalized, but needs to be marked as Approved), and I still need to finish my review.

The Skara bot can't know whether all criteria have been met. It can't, for example, know whether there are outstanding comments from some reviewers that need to be addressed. Nor does it know which PRs need two reviewers (or sometimes a third if there is a specific person we would like to review it), which ones need a CSR, etc.

So having it state authoritatively that the PR is ready to integrate is a bit misleading in this case. This is documented in the Code Review section of the CONTRIBUTING doc:

NOTE: while the Skara tooling will indicate that the PR is ready to integrate once the first reviewer with a "Reviewer" role in the project has approved it, this may or may not be sufficient depending on the type of fix. For example, you must wait for a second approval for enhancements or high-impact bug fixes.

I wonder if there is a way to improve this?

Copy link
Member

kevinrushforth left a comment

Looks good.

Once the CSR is approved, this can be integrated.

@kevinrushforth
Copy link
Member

kevinrushforth commented Dec 18, 2019

@Maxoudela the CSR has been approved. Go ahead and /integrate this and I will sponsor it.

@Maxoudela
Copy link
Author

Maxoudela commented Dec 18, 2019

/integrate

@kevinrushforth
Copy link
Member

kevinrushforth commented Dec 18, 2019

I think you need to enter the integrate command "as-is" without the back-quotes. the / needs to be the first character.

Update: looks like it works as you typed it. No further action on your part is needed.

@openjdk
Copy link

openjdk bot commented Dec 18, 2019

@Maxoudela
Your change (at version 7981702) is now ready to be sponsored by a Committer.

@openjdk openjdk bot added the sponsor label Dec 18, 2019
@kevinrushforth
Copy link
Member

kevinrushforth commented Dec 18, 2019

/sponsor

@openjdk openjdk bot closed this Dec 18, 2019
@openjdk openjdk bot added integrated and removed sponsor labels Dec 18, 2019
@openjdk
Copy link

openjdk bot commented Dec 18, 2019

@kevinrushforth @Maxoudela The following commits have been pushed to master since your change was applied:

  • 4e005e4: 8227808: Make GTK3 libraries mandatory for building on Linux
  • 1140d34: 8196587: Remove use of deprecated finalize method from JPEGImageLoader
  • d2d44b4: 8207759: VK_ENTER not consumed by TextField when default Button exists
  • fc539b5: 8223296: NullPointerException in GlassScene.java at line 325
  • 71ca899: 8220722: ProgressBarSkin: adds strong listener to control's width property
  • 07af89a: 8221334: TableViewSkin: must initialize flow's cellCount in constructor
  • 4ddf554: 8234916: [macos 10.15] Garbled text running with native-image
  • 46338d0: 8232524: SynchronizedObservableMap cannot be be protected for copying/iterating
  • a68347c: 8235150: IosApplication does not pass the required object in _leaveNestedEventLoopImpl
  • 1c27fbd: 8210955: DOMTest::testEventListenerCascade fails
  • 2d4096a: 8235151: Nonexistent notifyQuit method referred from iOS GlassHelper.m
  • 98035cb: 8211308: Support HTTP/2 in WebView
  • 6892fa1: 8232064: Switch FX build to use JDK 13.0.1 as boot JDK
  • 1d670f1: 8200224: Multiple press event when JFXPanel gains focus
  • 83eb0a7: 8193445: JavaFX CSS is applied redundantly leading to significant performance degradation
  • 798afbc: 8230610: Upgrade GStreamer to version 1.16.1
  • 126896d: 8234704: Fix attribution in libxslt.md
  • 4d3c723: 8234593: Mark LeakTest.testGarbageCollectability as unstable
  • 5a39824: 8234056: Upgrade to libxslt 1.1.34
  • 8bea7b7: 8229472: Deprecate for removal JavaBeanXxxPropertyBuilders constructors
  • aad1720: 8233420: Upgrade to gcc 8.3 on Linux
  • 42040c4: 8232063: Upgrade gradle to version 6.0
  • aab07a4: 8234239: [TEST_BUG] Reenable few ignored web tests
  • 95ad601: 8233421: Upgrade to Visual Studio 2017 version 15.9.16
  • 3e0557a: 8234303: [TEST_BUG] Correct ignore tag in graphics unit tests
  • e37cb37: 8234150: Address ignored tests in ComboBoxTest, LabeledTest, HyperLinkTest and TextInputControlTest
  • 4f496d4: 8234194: [TEST_BUG] Reenable few graphics unit tests
  • 927fc8a: 8234174: Change IDEA VCS mapping to Git
  • 3d0cb49: 8234189: [TEST_BUG] Remove ignored and invalid graphics unit tests
  • dc01309: 8234110: SwingFXUtilsTest is unsuitable for unit test framework
  • 5b96ee4: 8231188: Update SQLite to version 3.30.1
  • d46dcae: 8233338: FX javadoc headings are out of sequence
  • 94bcf3f: 8231692: Test Infrastructure: enhance KeyEventFirer to inject keyEvents into scene
  • 286d1b5: 8230492: font-family not set in HTMLEditor if font name has a number in it
  • f74f3af: 8233040: ComboBoxPopupControl: remove eventFilter for F4
  • a1cc4ab: 8232210: Update Mesa 3-D Headers to version 19.2.1
  • dca8df4: 8232943: Gesture support is not initialized on iOS
  • 5a70b0c: 8189092: ArrayIndexOutOfBoundsException on Linux in getCachedGlyph
  • ac71396: 8232929: Duplicate symbols when building static libraries
  • ab6ea3b: 8232158: [macOS] Fallback to command line tools if xcode is missing
  • 2ae171a: 8232687: No static JNI loader for libprism-sw
  • a09a0fa: 8232522: FX: Update copyright year in docs, readme files to 2020
  • 582d999: Merge
  • b6e53f4: 8218640: Update ICU4C to version 64.2
  • bada612: Merge
  • 64aaeb8: 8226754: FX build fails using gradle 5.6+ or 6
  • 63fe66c: 8231870: CrossLibs script for armv6hf toolchain download fails
  • a4bc22d: Merge
  • c6eb091: 8231854: Change Mercurial to git in various README files
  • 0a0d34a: 8231590: Update location of jfx repo to GitHub in third-party legal files
  • f595cc1: 8231735: gradle checkrepo is obsolete and doesn't work with git
  • 2593dea: Merge
  • 9df20c5: Merge
  • a1aa38a: Merge
  • 0ff02bb: Merge
  • a433bf2: Merge
  • 561153d: Merge
  • 75d439e: Merge
  • 758252f: Merge
  • 35e0cae: 8227402: Improve XSLT processing

Your commit was automatically rebased without conflicts.

Pushed as commit 935e99d.

@openjdk openjdk bot removed the ready label Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.