refactor: migrate to zio-json + update all #438

ThijsBroersen · 2024-10-17T20:21:56Z

Why circe if we have zio-json? 😏

I think main is broken because some PR's got merged which did not pass CI. Like circe-yaml 0.16.0 which is not available for Scala 2.12 anymore.

This PR updates all dependencies and migrates to zio-json. I see some other areas where improvements can be made but will target that in separate PR's.

I ran sbt prepare as in the Contributing guide but not sbt testPlugin that seems broken. I could not find any reference to the testPlugin task. I also could not find any tests in the repo... how is this project tested? Manually?

With zio-json I created codecs (both encoders and decoders) so perhaps validations can now also be done by parsing the yaml and decoding back to the model and then create a set of rules for it. Perhaps even allow manual tweaks and on each new generate-run first load the existing flow and then update this flow.

ThijsBroersen · 2024-10-17T20:56:37Z

The PR is btw breaking because it changes the model so the codecs become easier because they match the GHA model more closely. Any opinions?
I think it helps to migrate and not have to remember different views on the same thing.

--update--
I restored the existing model and added a new one. With toNative methods to convert. I tested it on a few zio projects and the required changes are quite small.

ThijsBroersen · 2024-10-17T21:11:48Z

zio-sbt-ecosystem/src/main/scala/zio/sbt/ScalaCompilerSettings.scala

@@ -190,7 +190,7 @@ trait ScalaCompilerSettings {
            )
          } else Seq.empty
        },
-        Test / parallelExecution := scalaBinaryVersion.value != "3",
+        Test / parallelExecution := scalaBinaryVersion.value != "3", // why not parallel execution for Scala 3?


Discovered this setting.. why was this set? I would prefer to not have this set.

It's good to remove it! However, I don't remember why this line was included. Can you check to ensure it's okay to remove it?

ThijsBroersen · 2024-10-17T21:12:37Z

zio-sbt-ecosystem/src/main/scala/zio/sbt/ScalaCompilerSettings.scala

@@ -233,8 +233,7 @@ trait ScalaCompilerSettings {
           libraryDependencies ++= Seq(
             "dev.zio" %%% "zio-test"     % ZioSbtEcosystemPlugin.autoImport.zioVersion.value % Test,
             "dev.zio" %%% "zio-test-sbt" % ZioSbtEcosystemPlugin.autoImport.zioVersion.value % Test
-           ),
-           testFrameworks += new TestFramework("zio.test.sbt.ZTestFramework")
+           )


For quite some time ZTestFramework is added to testFrameworks in sbt by default.

guizmaii · 2024-10-19T23:53:42Z

project/Versions.scala

+  val Scala212 = "2.12.20"
+  val Scala213 = "2.13.15"
+  val Scala3   = "3.3.4"
+  val zio      = "2.1.9"


Suggested change

val zio = "2.1.9"

val zio = "2.1.11"

guizmaii · 2024-10-19T23:54:09Z

project/plugins.sbt

-libraryDependencies += "dev.zio"      %% "zio"              % "2.1.8"
-libraryDependencies += "io.circe"     %% "circe-yaml"       % "0.16.0"
+libraryDependencies += "org.snakeyaml" % "snakeyaml-engine" % "2.8"
+libraryDependencies += "dev.zio"      %% "zio"              % "2.1.9"


Suggested change

libraryDependencies += "dev.zio" %% "zio" % "2.1.9"

libraryDependencies += "dev.zio" %% "zio" % "2.1.11"

guizmaii · 2024-10-19T23:54:16Z

zio-sbt-ci/build.sbt

@@ -1,2 +1,3 @@
-libraryDependencies += "dev.zio"  %% "zio"        % "2.1.8"
-libraryDependencies += "io.circe" %% "circe-yaml" % "0.16.0"
+libraryDependencies += "dev.zio" %% "zio"           % "2.1.9"


Suggested change

libraryDependencies += "dev.zio" %% "zio" % "2.1.9"

libraryDependencies += "dev.zio" %% "zio" % "2.1.11"

guizmaii · 2024-10-19T23:54:31Z

zio-sbt-ecosystem/build.sbt

-libraryDependencies += "org.snakeyaml" % "snakeyaml-engine" % "2.7"
-libraryDependencies += "dev.zio"      %% "zio"              % "2.1.8"
-libraryDependencies += "io.circe"     %% "circe-yaml"       % "0.16.0"
+libraryDependencies += "dev.zio" %% "zio"           % "2.1.9"


Suggested change

libraryDependencies += "dev.zio" %% "zio" % "2.1.9"

libraryDependencies += "dev.zio" %% "zio" % "2.1.11"

guizmaii · 2024-10-19T23:55:00Z

zio-sbt-ecosystem/src/main/scala/zio/sbt/Versions.scala


-  val zioVersion = "2.0.21"
+  val zioVersion = "2.1.9"


Suggested change

val zioVersion = "2.1.9"

val zioVersion = "2.1.11"

guizmaii · 2024-10-19T23:55:14Z

zio-sbt-githubactions/build.sbt

@@ -1,2 +1,3 @@
-libraryDependencies += "dev.zio"  %% "zio"        % "2.1.8"
-libraryDependencies += "io.circe" %% "circe-yaml" % "0.16.0"
+libraryDependencies += "dev.zio" %% "zio"           % "2.1.9"


Suggested change

libraryDependencies += "dev.zio" %% "zio" % "2.1.9"

libraryDependencies += "dev.zio" %% "zio" % "2.1.11"

guizmaii · 2024-10-19T23:56:50Z

zio-sbt-githubactions/src/main/scala/zio/sbt/githubactionsnative/ScalaWorkflow.scala

+
+// The original code of the githubactions package was originally copied from the zio-aws-codegen project:
+// https://github.com/zio/zio-aws/tree/master/zio-aws-codegen/src/main/scala/zio/aws/codegen/githubactions
+object ScalaWorkflow {


What's the need to rewrite this code from scratch?

I added the new solution next to the existing. But I think in the end result the old model and workflow are not used anymore in the other modules.
Perhaps the question should be: should we use the new model, stick with the old or somehow support both?
My reasons for the new model is that it is closer to writing direct GHA yaml and less of a cognitive load (no need to learn another DSL). It also simplifies the codecs because we reduce the amount of codec adjustments or required DTO's.

I would love to contribute some more but I really think a model which is as much 1:1 with GHA model as possible, helps to build better workflows. It is predictable and the codecs are easier.

ThijsBroersen · 2024-11-27T20:13:33Z

The ScalaWorkflow now only has a single version.

khajavi · 2024-11-28T10:56:48Z

zio-sbt-ci/src/main/scala/zio/sbt/V.scala

@@ -5,7 +5,7 @@ object V {
    Map(
      "peter-evans/create-pull-request" -> "v6",
      "zio/generate-github-app-token"   -> "v1.0.0",
-      "pierotofy/set-swap-space"        -> "master",
+      "pierotofy/set-swap-space"        -> "49819abfb41bd9b44fb781159c033dba90353a7c", // 1.0,


maybe it is better to mention the version here: "v1.0"

I was not sure about the correct tagging. The github tag is 1.0 and the marketplace says v1.0. I just wanted to prevent the action from changing. I can try and make it v1.0 or 1.0.

khajavi · 2024-11-28T11:06:49Z

zio-sbt-githubactions/src/main/scala/zio/sbt/githubactions/model.scala

    run: Option[String] = None,
-    env: Map[String, String] = Map.empty
+    env: Option[ListMap[String, String]] = None


Why not env: ListMap[String, String] = ListMap.empty?

Optional keys are automatically left out of the json object. An empty map will be encoded as {}. I do not think zio-json has the option to exclude keys with empty objects yet.

Perhaps in zio-json this could be fixed with a default value. Dropping the key when the value is the default value.

I prefer to make it as simple as possible! So probably it should be fixed on zio-json side.

See zio/zio-json#1193

khajavi · 2024-11-28T16:06:59Z

zio-sbt-githubactions/src/main/scala/zio/sbt/githubactions/model.scala

+
+  case object UbuntuLatest extends OS("ubuntu-latest")
+  case object Ubuntu2404   extends OS("ubuntu-24.04")
+  case object Ubuntu2204   extends OS("ubuntu-22.04")


khajavi · 2024-11-28T16:19:55Z

zio-sbt-ci/src/main/scala/zio/sbt/ZioSbtCiPlugin.scala

-            ) ++ checkAllCodeCompiles ++ checkArtifactBuildProcess ++ checkWebsiteBuildProcess
+            ) ++ checkAllCodeCompiles.flatMap(_.flatten) ++ checkArtifactBuildProcess.flatMap(
+              _.flatten
+            ) ++ checkWebsiteBuildProcess.flatMap(_.flatten)


Why this boilerplate change is needed? Can you keep it as simple as it is?

yes, I made a custom codec for Seq[Step] in Job

khajavi · 2024-11-28T16:21:52Z

zio-sbt-ci/src/main/scala/zio/sbt/ZioSbtCiPlugin.scala

@@ -177,7 +178,7 @@ object ZioSbtCiPlugin extends AutoPlugin {
                  scalaVersionMatrix.values.toSeq.flatten.distinct.map { scalaVersion: String =>
                    Step.SingleStep(
                      name = "Test",
-                      condition = Some(Condition.Expression(s"matrix.scala == '$scalaVersion'")),
+                      `if` = Some(Condition.Expression(s"matrix.scala == '$scalaVersion'")),


Can we preserve the previous syntax? We can deprecate it, but make it possible to compile old codes.

I tried but because of all the default values there are already issues with overloaded constructors. If the zio-json PR will get merged I could transform a lot of optional sequences to plain sequences with the explicitEmptyCollections set to false.

khajavi · 2024-11-28T16:24:01Z

zio-sbt-ci/src/main/scala/zio/sbt/ZioSbtCiPlugin.scala

            }
          )
      )

    val DefaultTestStrategy =
      Job(
-        id = "test",


Why the id is removed?

The name is used, transformed to an id. This way we do not have to exclude the id field from the json. Do you prefer the explicit id field?

khajavi · 2024-11-28T16:35:54Z

zio-sbt-githubactions/src/main/scala/zio/sbt/githubactions/ScalaWorkflow.scala

+  sealed trait JavaVersion {
+    def distribution: String
+    def version: String
+    def asString: String = s"$distribution:$version"


ThijsBroersen · 2024-12-13T09:08:45Z

zio-sbt-ci/src/main/scala/zio/sbt/ZioSbtCiPlugin.scala

+      val yaml: String = zio.json.ast.Json.decoder
+        .decodeJson(workflow.toJson)


this is a bit dirty but the toJsonAST does not perform the excludeEmptyCollections , will investigate this later in zio-json

ThijsBroersen changed the title ~~refactor: migrate to zio-json~~ refactor: migrate to zio-json + update all Oct 17, 2024

ThijsBroersen mentioned this pull request Oct 17, 2024

Feature: support sbt-projectmatrix #439

Open

refactor: migrate to zio-json

b2a7d1a

ThijsBroersen force-pushed the refactor/migrate-to-zio-json branch from 7a9a6c8 to b2a7d1a Compare October 17, 2024 20:27

ThijsBroersen commented Oct 17, 2024

View reviewed changes

ThijsBroersen force-pushed the refactor/migrate-to-zio-json branch from ba39433 to eb7e7a8 Compare October 18, 2024 21:38

guizmaii reviewed Oct 19, 2024

View reviewed changes

ThijsBroersen mentioned this pull request Nov 3, 2024

Feat/support projectmatrix and support better crossbuilding #440

Open

ThijsBroersen closed this Nov 16, 2024

updates and dedup scalaworkflow

3fe9f88

ThijsBroersen reopened this Nov 27, 2024

ThijsBroersen force-pushed the refactor/migrate-to-zio-json branch from 97f3900 to 3fe9f88 Compare November 27, 2024 20:10

khajavi reviewed Nov 28, 2024

View reviewed changes

ThijsBroersen added 2 commits November 28, 2024 23:59

restore step sequence

223d0f2

add setup-sbt

e7dec54

use zio-json explicitEmptyCollections

28c49c4

ThijsBroersen commented Dec 13, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: migrate to zio-json + update all #438

refactor: migrate to zio-json + update all #438

ThijsBroersen commented Oct 17, 2024

ThijsBroersen commented Oct 17, 2024 •

edited

Loading

ThijsBroersen Oct 17, 2024

khajavi Nov 28, 2024

ThijsBroersen Oct 17, 2024

guizmaii Oct 19, 2024

guizmaii Oct 19, 2024

guizmaii Oct 19, 2024

guizmaii Oct 19, 2024

guizmaii Oct 19, 2024

guizmaii Oct 19, 2024

guizmaii Oct 19, 2024 •

edited

Loading

ThijsBroersen Oct 20, 2024

ThijsBroersen Oct 25, 2024

ThijsBroersen commented Nov 27, 2024

khajavi Nov 28, 2024

ThijsBroersen Nov 28, 2024

khajavi Nov 28, 2024

ThijsBroersen Nov 28, 2024

ThijsBroersen Nov 28, 2024

khajavi Nov 28, 2024

ThijsBroersen Nov 28, 2024

khajavi Nov 28, 2024

khajavi Nov 28, 2024

ThijsBroersen Nov 28, 2024

khajavi Nov 28, 2024

ThijsBroersen Nov 28, 2024

khajavi Nov 28, 2024

ThijsBroersen Nov 28, 2024

khajavi Nov 28, 2024

ThijsBroersen Dec 13, 2024 •

edited

Loading

	libraryDependencies += "dev.zio" %% "zio" % "2.1.9"
	libraryDependencies += "dev.zio" %% "zio" % "2.1.11"

		val yaml: String = zio.json.ast.Json.decoder
		.decodeJson(workflow.toJson)

refactor: migrate to zio-json + update all #438

Are you sure you want to change the base?

refactor: migrate to zio-json + update all #438

Conversation

ThijsBroersen commented Oct 17, 2024

ThijsBroersen commented Oct 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guizmaii Oct 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThijsBroersen commented Nov 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThijsBroersen Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

ThijsBroersen commented Oct 17, 2024 •

edited

Loading

guizmaii Oct 19, 2024 •

edited

Loading

ThijsBroersen Dec 13, 2024 •

edited

Loading