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

chore/refactor to projectmatrix #1466

Draft
wants to merge 7 commits into
base: series/4.x
Choose a base branch
from

Conversation

ThijsBroersen
Copy link

@ThijsBroersen ThijsBroersen commented Oct 6, 2024

PR to update about all...
Build has been migrated to sbt-projectmatrix because the existing build had quite some issues and did target all platforms and versions correctly.

I also noticed some gaps. Like annotations not implemented for Scala 3.

The JDK version is updated to 17 setting the jvm release target to 11.

Fixes/enables scalafix on CI (which is enabled by default with zio-sbt-ci)

@ThijsBroersen ThijsBroersen requested a review from a team as a code owner October 6, 2024 13:43
@ThijsBroersen ThijsBroersen force-pushed the chore/build/refactor-to-projectmatrix-and-bump-all branch 2 times, most recently from f6b944c to b5c4abc Compare October 6, 2024 17:30
@ThijsBroersen
Copy link
Author

I reverted the zio-sbt-website version. As the new version added a +publishLocal step which causes an issue when using projectmatrix. It needs to be without the +, I did not know how to override it.
For now it targets only Scala 2.13. I think this is also what main did as 2.13 was the default scalaVersion.

@@ -42,7 +42,7 @@ object AutomaticConfig extends App {
"aws.credentials.Token.value" -> "token",
"port" -> "10",
"default" -> "12",
"dburl" -> "some url",
"dburl.value" -> "some url",
Copy link
Author

Choose a reason for hiding this comment

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

There is some inconsistency on the usage of value-classes. Sometimes AnyVal is extended, sometimes not. In some tests it expects the path to have the nested path, but in this code it did not.
I think also some apps were not working in in the main branch. This change for now makes it work.

@ThijsBroersen ThijsBroersen force-pushed the chore/build/refactor-to-projectmatrix-and-bump-all branch 2 times, most recently from 9b11812 to dc0f710 Compare October 9, 2024 11:51
addSbtPlugin("pl.project13.scala" % "sbt-jcstress" % "0.2.0")
addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.4.7")
addSbtPlugin("dev.zio" % "zio-sbt-website" % "0.3.1")
addSbtPlugin("com.eed3si9n" % "sbt-projectmatrix" % "0.10.0")

Choose a reason for hiding this comment

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

I am not sure about this. I don't think any other zio project is using this so far. And we have zio-sbt to help us with the builds. I'd prefer to stick to the same solution, unless you can make a compelling argument.

Copy link
Author

Choose a reason for hiding this comment

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

This sbt-projectmatrix makes the build much nicer to work with imo. It is more explicit and helps to discover what platforms and versions are in scope. I have seen multiple public projects already using it and I am using it in all of my customer projects. It make it much easier to crossbuild, export all in a single bloopInstall instead of just one Scala version.

Copy link
Author

Choose a reason for hiding this comment

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

To my understanding it should also be a default feature in SBT 2.x

Copy link
Author

Choose a reason for hiding this comment

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

With the existing build and the command aliases to target the different platforms and scala versions it was quite error prone to make sure all projects were included. Now you do not have to use the + or ++ anymore. All projects have a direct projectref in the build.

Copy link
Author

Choose a reason for hiding this comment

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

I think the build is much more concise now and it is targeting all the platforms and Scala versions it can support (the existing build was missing some).

Copy link
Author

Choose a reason for hiding this comment

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

ref sbt-projectmatrix insource in sbt 2.x: https://eed3si9n.com/sbt-2.0.0-beta

addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.4.4")
addSbtPlugin("dev.zio" % "zio-sbt-website" % "0.3.1")

libraryDependencies += "org.snakeyaml" % "snakeyaml-engine" % "2.6"

Choose a reason for hiding this comment

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

was unused? or is now a transitive dependency?

Copy link
Author

Choose a reason for hiding this comment

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

It was used to extract Scale versions from the ci.yml definition. There is quite some difference on how different zio-projects are being configured. The BuildHelper.scala has a lot if different implementations. I think the Scala versions should just be part of the build files and not extracted from ci.yml. Most projects do this and it prevent us from requiring this dependency in the sbt tasks.

@ThijsBroersen ThijsBroersen marked this pull request as draft November 15, 2024 21:16
@ThijsBroersen ThijsBroersen changed the title chore/build/refactor-to-projectmatrix-and-upgrade-all-dependencies chore//refactor to projectmatrix Nov 15, 2024
@ThijsBroersen ThijsBroersen changed the title chore//refactor to projectmatrix chore/refactor to projectmatrix Nov 15, 2024
@ThijsBroersen ThijsBroersen marked this pull request as ready for review November 15, 2024 22:58
@ThijsBroersen ThijsBroersen marked this pull request as draft November 15, 2024 22:58
@ThijsBroersen ThijsBroersen force-pushed the chore/build/refactor-to-projectmatrix-and-bump-all branch 2 times, most recently from b0a9848 to 325fd63 Compare November 15, 2024 23:18
@ThijsBroersen ThijsBroersen force-pushed the chore/build/refactor-to-projectmatrix-and-bump-all branch from 6ef6b67 to b6c8f2b Compare November 16, 2024 17:11
@ThijsBroersen ThijsBroersen force-pushed the chore/build/refactor-to-projectmatrix-and-bump-all branch from b5732c1 to 4cd168e Compare November 17, 2024 21:22
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