Skip to content

Fix JDK 8 compatibility#530

Merged
popematt merged 1 commit intoamazon-ion:masterfrom
popematt:master
Jul 18, 2023
Merged

Fix JDK 8 compatibility#530
popematt merged 1 commit intoamazon-ion:masterfrom
popematt:master

Conversation

@popematt
Copy link
Copy Markdown
Contributor

Issue #, if available:

None

Description of changes:

Setting the target and source compatibility compiler options to JDK 8 is insufficient for ensuring JDK 8 compatibility of the JAR that we publish. We need to set the release option, which is not supported by JDK 8 javac, so we're dropping support for building with JDK 8.

See https://saker.build/blog/javac_source_target_parameters/index.html

This is a two way door. In theory, it is possible to add some conditional logic to the Gradle build script to only set this option when compiling with JDK 9 or higher, but I don't think we have a compelling reason for doing that right now.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@popematt popematt requested a review from jobarr-amzn July 18, 2023 21:02
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 18, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (c4a545e) 66.81% compared to head (0525fe8) 66.82%.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #530   +/-   ##
=========================================
  Coverage     66.81%   66.82%           
+ Complexity     5394     5391    -3     
=========================================
  Files           156      156           
  Lines         22724    22734   +10     
  Branches       4082     4081    -1     
=========================================
+ Hits          15184    15191    +7     
- Misses         6243     6246    +3     
  Partials       1297     1297           

see 11 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@popematt
Copy link
Copy Markdown
Contributor Author

popematt commented Jul 18, 2023

Note that the test (11) and test (8, true) checks will not run because I updated the workflow. If this PR is approved, I will also update the required workflows to test (11, true) and test (17).

steps:
- name: Set up JDK 1.8
uses: actions/setup-java@v1
- name: Set up JDK 11
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there any point in naming this with the version when that is redundant?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You can easily read it from the GHA status page, I suppose. 🤷‍♂️

@popematt popematt merged commit 1a2085d into amazon-ion:master Jul 18, 2023
@popematt popematt mentioned this pull request Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants