Skip to content

feat: Require Java 11 as minimum#588

Merged
patriknw merged 2 commits intomainfrom
wip-jdk11-patriknw
Oct 11, 2021
Merged

feat: Require Java 11 as minimum#588
patriknw merged 2 commits intomainfrom
wip-jdk11-patriknw

Conversation

@patriknw
Copy link
Copy Markdown
Contributor

@patriknw patriknw commented Oct 8, 2021

Comment thread build.sbt Outdated
.settings(
name := "akkaserverless-jvm-sdk",
crossPaths := false,
crossPaths := true,
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.

I changed this for core. Not sure if that is the right thing to do? I don't think we have a solid solution for cross building the Scala SDK anyway because it has a dependency to Java SDK, which doesn't have crossPaths enabled.

core itself doesn't have any Scala classes yet

Maybe I'll just change this back to false and we deal with the full story when/if there is a need for cross building?

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.

Also not sure, but true is the default so if we want it true, maybe remove it instead.

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.

maybe we shouldn't have an core at all, since scala-sdk depends on java-sdk and core only contains a single class (sdk/core/src/main/java/com/akkaserverless/replicatedentity/ReplicatedData.java)?

As long as core only has Java sources crossPaths=false might make sense, not sure.

Since java-sdk contains Scala code, shouldn't we change to crossPaths=true there as well?

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.

Do we even support anything else than 2.13 right now?

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.

Do we even support anything else than 2.13 right now?

Only 2.13 now, but I can imagine Scala 3 would come up later.

Since java-sdk contains Scala code, shouldn't we change to crossPaths=true there as well?

The drawback is that the dependency for the Java users becomes _2.13, but I agree that if we ever want to support cross builds the java-sdk must also be be crossPaths=true.

Let's deal with this later when there is a need for cross building.

Seq(
Compile / akkaGrpcGeneratedLanguages := Seq(AkkaGrpc.Java),
Test / akkaGrpcGeneratedSources := Seq(AkkaGrpc.Client),
Compile / javacOptions ++= Seq("-encoding", "UTF-8", "-source", "11", "-target", "11"),
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.

-source was conflicting with -release

This is only used from one of the test projects, but anyway it seems wrong that a plugin like this should set these settings

Copy link
Copy Markdown
Contributor

@johanandren johanandren left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread build.sbt Outdated
.settings(
name := "akkaserverless-jvm-sdk",
crossPaths := false,
crossPaths := true,
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.

Also not sure, but true is the default so if we want it true, maybe remove it instead.

@patriknw patriknw merged commit ebd1744 into main Oct 11, 2021
@patriknw patriknw deleted the wip-jdk11-patriknw branch October 11, 2021 12:14
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