-
Notifications
You must be signed in to change notification settings - Fork 6
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
Target JDK 1.8 and use sbt-github-actions for snapshot publish #97
Target JDK 1.8 and use sbt-github-actions for snapshot publish #97
Conversation
ThisBuild / githubWorkflowPublishTargetBranches := Seq() | ||
ThisBuild / githubWorkflowPublish := Seq() | ||
ThisBuild / githubWorkflowPublishTargetBranches := Seq( | ||
RefPredicate.Equals(Ref.Branch("main"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ensures that we only run +publish
when a PR is merged into main, i.e. snapshots (see https://github.com/sbt/sbt-github-actions?tab=readme-ov-file#integration-with-sbt-ci-release for instructions).
ThisBuild / githubWorkflowPublishTargetBranches := Seq( | ||
RefPredicate.Equals(Ref.Branch("main"))) | ||
|
||
ThisBuild / githubWorkflowPublish := Seq( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is exactly the same as the original publish.yml
.
a74036a
to
c87feaf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm - just a question about the windows build stuff
if: matrix.java == 'temurin@21' | ||
- name: Configure pagefile for Windows | ||
if: contains(runner.os, 'windows') | ||
uses: al-cheb/[email protected] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why all this windows stuff? not a blocker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just making sure that the plugin can be built on windows for people such as @He-Pin . The specific code you are pointing out is a workaround for how windows handles page files (tl;dr due to limited resources of github actions runners its possible to run out of memory specifically for windows).
This PR does 2 things, the first is it makes the project target JDK 1.8 and the second is that it makes the publish step be generated by sbt-github-actions rather than manually.
First thing to note is that even though this project is being changed to target JDK 1.8 (which is actually what we use to generate docs currently) its still perfectly possible to use the exact same binary with JDK 11+, you just need to add the newer sbt-paradox/sbt-paradox-theme versions (see https://github.com/sbt/sbt-paradox-material-theme/blob/main/src/main/paradox/getting-started.md#getting-started for more info). Note that it is actually intended that sbt-paradox-material-theme will continue targeting JDK 1.8/sbt-paradox 0.9.2 until its actually forced to update the version of sbt-paradox (i.e. sbt-paradox/sbt-paradox-theme will add some incompatible API which sbt-paradox-material-theme will need to use).
Changing the build to target JDK 1.8 was necessary in order to use sbt-github-actions to handle publishing (before this was a manual github workflow file). This is because sbt-github-actions uses the first value of
githubWorkflowJavaVersions
for publishing and since sbt-paradox 0.10.6 ONLY works with JDK 11+ it makes sense to both test for JDK 1.8 and use JDK 1.8 for publishing.tl;dr
After this PR is merged we can then add a
.scala-steward.conf
and add this project to scala-steward so that it has less maintenance burden