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

Automated releases using gha-scala-library-release-workflow #135

Merged
merged 9 commits into from
Apr 17, 2024

Conversation

andrew-nowak
Copy link
Member

@andrew-nowak andrew-nowak requested a review from a team as a code owner April 16, 2024 13:30
Copy link
Member

@rtyley rtyley left a comment

Choose a reason for hiding this comment

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

Supporting older versions of Java

You'll probably want to add "-release:11" to the scalacOptions here:

scalacOptions ++= Seq(
"-feature",
"-deprecation",
// upgrade warnings to errors except deprecations
"-Wconf:cat=deprecation:ws,any:e"
),

This is because gha-scala-library-release-workflow runs with a very recent version of Java (eg Java 17, or soon Java 21 LTS), and by default this means your classes will be compiled for only that version of Java or above.

If you want pan-domain-authentication to work on app-servers running Java 11 - or even Java 8 (though I definitely don't recommend that!), you'll need to specify that in the build with the "-release:11" flag (see recommended sbt settings and eg here)

Artifact-producing - and non-artifact-producing - modules

The pre-existing use of publishArtifact is a bit confusing, and won't work well with the sbt-version-policy plugin:

publishArtifact := false

It's confusing because it's set to false in commonSettings (implying that's a value setting that almost all sub-projects would want!), and then commonSettings is only used in a custom project() method which is used for some, but not all, of the sub-project definitions. To the outside observer, it's not obvious that using project() vs Project() will have that effect!

It won't work well with sbt-version-policy because the only flag that sbt-version-policy respects, when it's trying to work out whether it should download a previous version of an artifact for compatibility checking on a module, is publish / skip - so carefully applying publishArtifact := false in certain places isn't going to cut it!

If publish / skip isn't true, sbt-version-policy will assume that it needs to download a previous artifact for that module, and if that module doesn't actually publish artifacts, it will fail and crash the release (see scalacenter/sbt-version-policy#64).

There are only two projects that need publish / skip := true - the root project and exampleApp. Everything else can just have the default behaviour of publishing an artifact.

To try and make the confusing settings a bit clearer on other libraries, we're sometimes got rid of the name commonSettings etc, and instead used the name artifactProductionSettings, which has emphasised that the settings are only really required for sub-projects that actually produce artifacts.

build.sbt Outdated Show resolved Hide resolved
several dependent projects still run JDK 8, so specify that as the baseline for artifacts
required for the release option to work right
@andrew-nowak
Copy link
Member Author

@rtyley thanks, I think I've applied those suggestions now - we do still have dependents on java 8 (and I think Play 2.7) so will need it to stay JDK 8 compatible - I've set -release:8

I prefer commonSettings because it is common to all subprojects, including the example app which isn't destined to be published, so I've left it as that.

Copy link
Member

@rtyley rtyley 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 to me!

@andrew-nowak andrew-nowak merged commit b30caa1 into main Apr 17, 2024
1 check passed
@andrew-nowak andrew-nowak deleted the an/automated-release branch April 17, 2024 09:00
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