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

Enable using Kotlin in Ion Java #488

Merged
merged 10 commits into from
Nov 16, 2023
Merged

Conversation

popematt
Copy link
Contributor

@popematt popematt commented May 23, 2023

Issue #, if available:

Description of changes:

Changes, grouped by commit:

  • Update build to use Kotlin
    • There's a bunch of build logic that got added, but the key takeaway is that there's now a 3-stage jar process—compile, bundle & shadow, and finally minify
    • The r8 config rules are not obfuscating as a way of minifying, so stack traces are not affected.
    • Adds Gradle task to generate third-party attribution file for the published jar
    • Adds THIRD_PARTY_LICENSES.md file for the GitHub repo
    • Adds Gradle task to make sure that the THIRD_PARTY_LICENSES.md file is up-to-date with the actual state of reality.
  • Rewrite a class in Kotlin
    • This is mostly just to prove that the whole thing works
  • Fix ktlint violations
    • ktlint is an anti-bikeshedding linter for Kotlin. It's pretty opinionated about unused imports, and apparently it can detect them even in a Java file.

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

@popematt popematt marked this pull request as ready for review July 18, 2023 16:33
@popematt
Copy link
Contributor Author

Marking as "ready for review" so that it will run all of the workflows. Not actually ready for review yet.

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Attention: 17 lines in your changes are missing coverage. Please review.

Comparison is base (b08cbc4) 67.05% compared to head (2a193e7) 67.01%.

Files Patch % Lines
src/com/amazon/ion/impl/IonIteratorImpl.kt 56.41% 13 Missing and 4 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #488      +/-   ##
============================================
- Coverage     67.05%   67.01%   -0.05%     
+ Complexity     5462     5459       -3     
============================================
  Files           159      159              
  Lines         22987    22938      -49     
  Branches       4117     4113       -4     
============================================
- Hits          15415    15371      -44     
+ Misses         6295     6291       -4     
+ Partials       1277     1276       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@popematt popematt changed the title Proof of concept--using Kotlin in Ion Java Enable using Kotlin in Ion Java Nov 7, 2023
config/proguard/rules.pro Outdated Show resolved Hide resolved
@@ -17,6 +17,7 @@ jobs:
- java: 11
upload_reports: true
- java: 17
fail-fast: false
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If e.g. Java 17 task fails, it won't cancel the Java 11 task. I added it earlier on when we were still using JDK 8 and JDK 11, and there legitimately were reasons why it might fail for one and succeed for the other. I don't know if that's still possible between JDK 11 and JDK 17, but these tasks are relative short (matter of minutes) so if the task is already running why bother cancelling it? Just let it run.

build.gradle.kts Outdated Show resolved Hide resolved
build.gradle.kts Show resolved Hide resolved
build.gradle.kts Outdated Show resolved Hide resolved
@popematt popematt merged commit 7fe1fd8 into amazon-ion:master Nov 16, 2023
18 of 34 checks passed
tgregg pushed a commit that referenced this pull request Jan 18, 2024
tgregg pushed a commit that referenced this pull request Jan 18, 2024
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.

2 participants