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

[#166] Bump javafx to version 11.0.2 #177

Merged
merged 1 commit into from
Jun 3, 2023

Conversation

Eclipse-Dominator
Copy link
Contributor

@Eclipse-Dominator Eclipse-Dominator commented May 18, 2023

Fixes #166

Conflict between JavaFx version 11 and Ubuntu 18.04 and above described in JDK-8210411 presents a compatibility issue for users using Ubuntu 18.04 and above. This fatal crash also causes JVM to generate a hs_err_pid log file.

This issue is fixed in JavaFx version 11.0.2. Let's bump JavaFx to the appropriate version to avoid this issue.

@canihasreview
Copy link

canihasreview bot commented May 18, 2023

Click here to submit a new iteration when this PR is ready for review.

See this repository's contribution guide for more information.

@canihasreview
Copy link

canihasreview bot commented May 18, 2023

v1

@Eclipse-Dominator submitted v1 for review.

(📚 Archive)

Checkout this PR version locally
git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/177/1/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

Copy link
Contributor

@damithc damithc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 1:
The commit message (subject or body) does not specify what is the actual issue being fixed.
Include a brief description. In addition, if there is an online description of the problem, we can give a link.
described in JDK-8210411 presents a compatibility issue for users using is longer than 72 chars.

Commit 2:
Let's not sneak in unrelated changes into a PR. That reduces the searchability of the PR/issue tracker.

@Eclipse-Dominator Eclipse-Dominator force-pushed the 166-bump-javafx-ver branch 4 times, most recently from 540d484 to a6fc3c0 Compare May 18, 2023 11:52
@canihasreview
Copy link

canihasreview bot commented May 18, 2023

v2

@Eclipse-Dominator submitted v2 for review.

(📚 Archive) (📈 Interdiff between v1 and v2) (📈 Range-Diff between v1 and v2)

Checkout this PR version locally
git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/177/2/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

@canihasreview canihasreview bot requested a review from damithc May 18, 2023 11:53
@codecov-commenter
Copy link

codecov-commenter commented May 18, 2023

Codecov Report

Merging #177 (b7a2713) into master (481a3d9) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #177   +/-   ##
=========================================
  Coverage     72.06%   72.06%           
  Complexity      399      399           
=========================================
  Files            70       70           
  Lines          1235     1235           
  Branches        127      127           
=========================================
  Hits            890      890           
  Misses          314      314           
  Partials         31       31           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@damithc damithc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you tested this to confirm the fix works (I didn't).

Separately, check our JavaFX guide (https://se-education.org/guides/tutorials/javaFx.html) and the Grade guide to see if they need updating too.

Commit message subject is too generic?

@damithc damithc requested a review from zhongfu May 18, 2023 13:51
@damithc
Copy link
Contributor

damithc commented May 18, 2023

@zhongfu I remember you did some investigation into this problem earlier. If you are free, your inputs are welcome.

Copy link
Contributor

@zhongfu zhongfu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me; I can confirm that this PR allows AB3 to run on Wayland without needing to force the use of X11 explicitly

(p.s. sorry for the late reply -- I missed this somehow)

@damithc
Copy link
Contributor

damithc commented Jun 2, 2023

@zhongfu thanks very much for checking. 👍

Conflict between JavaFx version 11 and Ubuntu 18.04 and above
described in JDK-8210411 will cause JVM to crash for users
using Ubuntu 18.04 and above.
This bug is patched in version 11.0.2
(bugs.openjdk.org/browse/JDK-8210411)

Let's bump JavaFx to version 11.0.2 to avoid JVM crashes.
@canihasreview
Copy link

canihasreview bot commented Jun 2, 2023

v3

@Eclipse-Dominator submitted v3 for review.

(📚 Archive) (📈 Interdiff between v2 and v3) (📈 Range-Diff between v2 and v3)

Checkout this PR version locally
git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/177/3/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

@canihasreview canihasreview bot requested a review from damithc June 2, 2023 20:56
@damithc damithc merged commit 2215c93 into se-edu:master Jun 3, 2023
@Eclipse-Dominator Eclipse-Dominator deleted the 166-bump-javafx-ver branch June 3, 2023 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump javafx to version 11.0.2
4 participants