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

Scala Crossbuild of Client Library with Scala 2.13 (#93) #94

Merged
merged 3 commits into from
Jan 25, 2024

Conversation

TheLydonKing
Copy link
Collaborator

@TheLydonKing TheLydonKing commented Jan 24, 2024

Created a Package of the Client Library for Scala 2.13.

Please let me know if any other scala versions require compatibility?

Also included some general cleanup of imports and a change to the Build.sbt to avoid publishing of the root project to Sonatype.

@TheLydonKing TheLydonKing linked an issue Jan 24, 2024 that may be closed by this pull request
Copy link

JaCoCo code coverage report - scala:2.12.17

There is no coverage information present for the Files changed

Total Project Coverage 75.04% 🍏

@TheLydonKing TheLydonKing marked this pull request as ready for review January 24, 2024 12:01
Copy link
Collaborator

@dk1844 dk1844 left a comment

Choose a reason for hiding this comment

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

Looks good, I suggest a couple of improvements:

  • please update the build workflow so that the 2.13 library is also built and tested (maybe another step for clientLibrary crossbuilt with stb ++ ...targets
  • similarly, I think that the release workflow does not release 2.13 variant which is important. Again, either solve by ++ crossbuild directly or (if this failed for some reason, another step with 2.13 explicitly. Later on, we could test-release both 2.12 and 2.13 together

build.sbt Outdated
@@ -52,6 +60,7 @@ lazy val clientLibrary = project // no need to define file, because path is same
name := "login-service-client-library",
libraryDependencies ++= clientLibDependencies,
javacOptions ++= commonJavacOptions,
crossScalaVersions := Seq(scala212, "2.13.12")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, I suggest defining
lazy val scala213 = "2.13.12"

and then

Suggested change
crossScalaVersions := Seq(scala212, "2.13.12")
crossScalaVersions := Seq(scala212, scala213)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi, so I tested a local release and it does indeed publish the 2.13 version. I just haven't made a release tag yet since I'm waiting for this to be reviewed and merged before I do. ci-release uses +publish which publishes all the crossbuild versions. So no worries there.

Will have a look at testing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi, so I tested a local release and it does indeed publish the 2.13 version. I just haven't made a release tag yet since I'm waiting for this to be reviewed and merged before I do. ci-release uses +publish which publishes all the crossbuild versions. So no worries there.

Oh, that's great, I missed that.

@TheLydonKing
Copy link
Collaborator Author

@dk1844 Ran tests with the workflow change and it tests both scala versions for the Client Library. So above comments should be sorted.

Here are the logs showing the above:

Setting the Project to Scala 2.13:
image

Passed tests for Client Lib in 2.13:
image

Link to Log: https://github.com/AbsaOSS/login-service/actions/runs/7652532167/job/20852485618

Copy link
Collaborator

@dk1844 dk1844 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the updates

@TheLydonKing TheLydonKing merged commit b3cc5b9 into master Jan 25, 2024
3 checks passed
@TheLydonKing TheLydonKing deleted the feature/93-scala-crossbuild-add-scala-213 branch April 10, 2024 12:10
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.

Scala crossbuild - add Scala 2.13
2 participants