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

Refactor build script. #528

Merged

Conversation

overcat
Copy link
Member

@overcat overcat commented Sep 14, 2023

Changelog:

  • No longer provide shadow jar.
  • The jar provided by default does not include third-party dependencies, when you use Gradle and Maven, third-party dependencies should be automatically downloaded.
  • Provided an uber jar, which includes the third-party dependencies used.
  • Bump Gradle from 7.4.1 to 8.3
  • Migrate the build script to kotlin
  • Upgrade the JDK version used for building to JDK 17.
  • Bump Gradle plugins

@overcat overcat marked this pull request as ready for review September 19, 2023 00:26
publications {
create<MavenPublication>("mavenJava") {
from(components["java"])
artifact(tasks["uberJar"])
Copy link
Contributor

Choose a reason for hiding this comment

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

what will the artifact classifier be for the uber jar? Assuming the non-uber jar will be the artifact with classifier=jar correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Their classifiers are '', 'uber', and 'javadoc'.

Copy link
Member Author

Choose a reason for hiding this comment

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

Users can add them in three different ways.

    implementation("com.github.stellar:java-stellar-sdk:0.41.0")
    implementation("com.github.stellar:java-stellar-sdk:0.41.0:uber")
    implementation("com.github.stellar:java-stellar-sdk:0.41.0:javadoc")

Copy link
Contributor

Choose a reason for hiding this comment

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

awesome, would you mind just adding a note for this in pending section in the CHANGELOG.md, so users can choose between the legacy behavior with dependency of com.github.stellar:java-stellar-sdk:0.41.0:uber or if they want 'thin' jar and transitive dependency management then com.github.stellar:java-stellar-sdk:0.41.0

Copy link
Member Author

Choose a reason for hiding this comment

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

I have updated the changelog, but ubar jar is different from the previous shadow jar in that it does not relocate third-party dependencies.

Copy link
Contributor

@sreuland sreuland left a comment

Choose a reason for hiding this comment

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

this looks great, just wondering about how to describe this in CHANGELOG.md, its not necessarily breaking, for many sdk users, the dependencies should resolve at build time, but we should state if they have any explicit deps declared against same nested dependency tree as this project, then they may run into runtime classloader issue to be aware of?

@overcat
Copy link
Member Author

overcat commented Sep 19, 2023

this looks great, just wondering about how to describe this in CHANGELOG.md, its not necessarily breaking, for many sdk users, the dependencies should resolve at build time, but we should state if they have any explicit deps declared against same nested dependency tree as this project, then they may run into runtime classloader issue to be aware of?

I think they may encounter dependency conflicts, but I believe this can be resolved. Additionally, based on my testing, I successfully imported the thin jar from jitpack into an android template project without any conflicts.

# Conflicts:
#	build.gradle
#	src/main/java/org/stellar/sdk/responses/sorobanrpc/SimulateTransactionResponse.java
@sreuland sreuland merged commit be49025 into lightsail-network:soroban Sep 19, 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.

2 participants