Skip to content

Conversation

@popematt
Copy link
Contributor

Issue #, if available:

None

Description of changes:

  • Upgrades to Gradle 8.6
  • Changes build script from Groovy to Kotlin
  • Moves source code to the conventional Gradle locations (i.e. src/main/kotlin, src/test/kotlin, src/test/java).

The changes that actually need attention in the review are much smaller than it might seem.

  • The only files that were edited by me are:
    • build.gradle.kts (new)
    • .editorconfig (modified)
    • build.gradle (deleted)
  • All files in src/ or test/ were moved without changing any content.
  • Changes in all other files (.gitattributes, gradlew, gradlew.bat, everything in gradle/) were generated by running gradle init to set up the project using Gradle 8.6 with Kotlin script.

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

Comment on lines +31 to +32
api("com.amazon.ion:ion-java:[1.4.0,)")
compileOnly("com.amazon.ion:ion-java:1.4.0")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these minimum requirements be raised given some of the fixes we've recently made in ion-java?

Copy link
Contributor Author

@popematt popematt Apr 25, 2024

Choose a reason for hiding this comment

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

I don't know. Ion Element only uses the reader/writer APIs. In general, I think it's okay to allow users of ion-element-kotlin to choose any compatible version of ion-java.

We will probably need to revisit this once Ion 1.1 is done since there will probably be new APIs in ion-java that ion-element-kotlin should leverage.

@popematt popematt merged commit 9d24f82 into amazon-ion:master Apr 25, 2024
@popematt popematt deleted the refactor_layout branch April 25, 2024 23:34
@popematt popematt mentioned this pull request Sep 19, 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