stevedlawrence commented on code in PR #199:
URL: https://github.com/apache/daffodil-sbt/pull/199#discussion_r3461738319


##########
project/DaffodilScalaVersions.scala:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import sbt._
+
+// Scala versions to use for each supported Daffodil version range. Defined 
here in project/
+// so scala-steward can detect and update them. A sourceGenerators task in 
build.sbt embeds
+// these version strings into the compiled plugin.
+//
+// We use full ModuleIds rather than plain strings so scala-steward can detect 
and update the
+// versions. The .revision call extracts just the version string at build time.
+//
+// If we update one of the below scala-library versions, we should verify that 
official
+// Daffodil releases using older versions of scala-library can still reload 
saved parsers built
+// with this newer scalaVersion (see the comment in DaffodilPlugin.scala for 
details). See the
+// versions-01 scripted test for how to manually verify this.
+object DaffodilScalaVersions {
+  val scala3 = ("org.scala-lang" % "scala3-library" % "3.3.7").revision
+  // scala-steward:off
+  val scala213Pinned = ("org.scala-lang" % "scala-library" % 
"2.13.16").revision
+  // scala-steward:on
+  val scala212 = ("org.scala-lang" % "scala-library" % "2.12.21").revision

Review Comment:
   I think these variable names should reference the daffodil versions they go 
with. For example, it's possible Daffodil 4.0 and 4.x could require different 
versions of scala 3.3.x so we would need different `scala3` names, with one of 
them pinned.



##########
src/main/scala/org/apache/daffodil/DaffodilPlugin.scala:
##########
@@ -258,18 +258,13 @@ object DaffodilPlugin extends AutoPlugin {
      * to the working version, which may limit JDK support.
      */
     scalaVersion := {
-      // We really only need Scala versions and not full ModuleIds, but using 
ModuleId.revision
-      // makes it easier for scala-steward to detect and automatically update 
the versions when
-      // new Scala patch versions are released.
-      //
-      // Note that if we update one of the below scala-library versions, we 
should verify that
-      // official Daffodil releases using older versions of scala-library can 
still reload saved
-      // parsers built with this newer scalaVersion (see above note about 
serialization
-      // compatibility). See the versions-01 scripted test to how to manually 
verify this.
+      // Scala versions are defined in project/DaffodilScalaVersions.scala so 
that
+      // scala-steward can detect and update them. A sourceGenerators task in 
build.sbt
+      // embeds those version strings into DaffodilScalaVersions at compile 
time.
       val daffodilToScalaVersion = Map(
-        ">=4.0.0 " -> ("org.scala-lang" % "scala3-library" % "3.3.7").revision,
-        "=3.11.0 " -> ("org.scala-lang" % "scala-library" % 
"2.13.16").revision, // scala-steward:off
-        "<=3.10.0" -> ("org.scala-lang" % "scala-library" % "2.12.21").revision
+        ">=4.0.0 " -> DaffodilScalaVersions.scala3,
+        "=3.11.0 " -> DaffodilScalaVersions.scala213Pinned,
+        "<=3.10.0" -> DaffodilScalaVersions.scala212

Review Comment:
   I wonder if there'sa scala-steward config option we can use to tell it to 
look at this file? It would be nice if we didn't need all this indirection and 
source generators just to get automatic updates. Especially since I would 
expect scala-steward to work with sbt plugins since they often added 
dependencies.



##########
src/main/scala/org/apache/daffodil/DaffodilPlugin.scala:
##########
@@ -258,18 +258,13 @@ object DaffodilPlugin extends AutoPlugin {
      * to the working version, which may limit JDK support.
      */
     scalaVersion := {
-      // We really only need Scala versions and not full ModuleIds, but using 
ModuleId.revision
-      // makes it easier for scala-steward to detect and automatically update 
the versions when
-      // new Scala patch versions are released.
-      //
-      // Note that if we update one of the below scala-library versions, we 
should verify that
-      // official Daffodil releases using older versions of scala-library can 
still reload saved
-      // parsers built with this newer scalaVersion (see above note about 
serialization
-      // compatibility). See the versions-01 scripted test to how to manually 
verify this.
+      // Scala versions are defined in project/DaffodilScalaVersions.scala so 
that
+      // scala-steward can detect and update them. A sourceGenerators task in 
build.sbt
+      // embeds those version strings into DaffodilScalaVersions at compile 
time.
       val daffodilToScalaVersion = Map(
-        ">=4.0.0 " -> ("org.scala-lang" % "scala3-library" % "3.3.7").revision,
-        "=3.11.0 " -> ("org.scala-lang" % "scala-library" % 
"2.13.16").revision, // scala-steward:off
-        "<=3.10.0" -> ("org.scala-lang" % "scala-library" % "2.12.21").revision
+        ">=4.0.0 " -> DaffodilScalaVersions.scala3,

Review Comment:
   Need to confirm that 4.0.0 and 4.1.0 also work with Scala 3.3.8.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to