Roiocam commented on code in PR #1054: URL: https://github.com/apache/incubator-pekko/pull/1054#discussion_r1468517117
########## project/Jdk9.scala: ########## @@ -11,36 +11,87 @@ * Copyright (C) 2017-2022 Lightbend Inc. <https://www.lightbend.com> */ -import sbt._ import sbt.Keys._ +import sbt._ object Jdk9 extends AutoPlugin { import JdkOptions.notOnJdk8 + // The version 9 is special for any Java versions >= 9 + // and the version 11 is special for any Java versions >= 11 + // and the version 17 is special for any Java versions >= 17 + // and the version 21 is special for any Java versions >= 21 + private val supportedJavaLTSVersions = List("9", "11", "17", "21") + lazy val CompileJdk9 = config("CompileJdk9").extend(Compile) lazy val TestJdk9 = config("TestJdk9").extend(Test).extend(CompileJdk9) - val SCALA_SOURCE_DIRECTORY = "scala-jdk-9" - val SCALA_TEST_SOURCE_DIRECTORY = "scala-jdk9-only" - val JAVA_SOURCE_DIRECTORY = "java-jdk-9" - val JAVA_TEST_SOURCE_DIRECTORY = "java-jdk9-only" + lazy val ScalaSourceDirectories: Seq[String] = getAdditionalSourceDirectoryNames("scala", isTest = false) + lazy val ScalaTestSourceDirectories: Seq[String] = getAdditionalSourceDirectoryNames("scala", isTest = true) + + lazy val JavaSourceDirectories: Seq[String] = getAdditionalSourceDirectoryNames("java", isTest = false) + lazy val JavaTestSourceDirectories: Seq[String] = getAdditionalSourceDirectoryNames("java", isTest = true) + + private lazy val additionalScalaSourceDirectories = Def.setting { + getAdditionalSourceDirectories( Review Comment: I think these methods could be simpler. ########## project/CopyrightHeaderForJdk9.scala: ########## @@ -12,24 +12,25 @@ */ import de.heikoseeberger.sbtheader.HeaderPlugin.autoImport.headerSources -import sbt.Keys.sourceDirectory import sbt.{ Compile, Def, Test, _ } object CopyrightHeaderForJdk9 extends AutoPlugin { override lazy val requires = CopyrightHeader && Jdk9 override lazy val trigger = allRequirements - override lazy val projectSettings: Seq[Def.Setting[_]] = { + private lazy val additionalFiles = Def.setting { import Jdk9._ - Seq( - Compile / headerSources ++= - (((Compile / sourceDirectory).value / SCALA_SOURCE_DIRECTORY) ** "*.scala").get, - Test / headerSources ++= - (((Test / sourceDirectory).value / SCALA_TEST_SOURCE_DIRECTORY) ** "*.scala").get, - Compile / headerSources ++= - (((Compile / sourceDirectory).value / JAVA_SOURCE_DIRECTORY) ** "*.java").get, - Test / headerSources ++= - (((Test / sourceDirectory).value / JAVA_TEST_SOURCE_DIRECTORY) ** "*.java").get) + for { + dir <- additionalSourceDirectories.value ++ additionalTestSourceDirectories.value Review Comment: brilliant. ########## project/Jdk9.scala: ########## @@ -11,36 +11,87 @@ * Copyright (C) 2017-2022 Lightbend Inc. <https://www.lightbend.com> */ -import sbt._ import sbt.Keys._ +import sbt._ object Jdk9 extends AutoPlugin { import JdkOptions.notOnJdk8 + // The version 9 is special for any Java versions >= 9 + // and the version 11 is special for any Java versions >= 11 + // and the version 17 is special for any Java versions >= 17 + // and the version 21 is special for any Java versions >= 21 + private val supportedJavaLTSVersions = List("9", "11", "17", "21") + lazy val CompileJdk9 = config("CompileJdk9").extend(Compile) lazy val TestJdk9 = config("TestJdk9").extend(Test).extend(CompileJdk9) - val SCALA_SOURCE_DIRECTORY = "scala-jdk-9" - val SCALA_TEST_SOURCE_DIRECTORY = "scala-jdk9-only" - val JAVA_SOURCE_DIRECTORY = "java-jdk-9" - val JAVA_TEST_SOURCE_DIRECTORY = "java-jdk9-only" + lazy val ScalaSourceDirectories: Seq[String] = getAdditionalSourceDirectoryNames("scala", isTest = false) + lazy val ScalaTestSourceDirectories: Seq[String] = getAdditionalSourceDirectoryNames("scala", isTest = true) + + lazy val JavaSourceDirectories: Seq[String] = getAdditionalSourceDirectoryNames("java", isTest = false) + lazy val JavaTestSourceDirectories: Seq[String] = getAdditionalSourceDirectoryNames("java", isTest = true) + + private lazy val additionalScalaSourceDirectories = Def.setting { + getAdditionalSourceDirectories( + (Compile / sourceDirectory).value, + ScalaSourceDirectories) + } + + private lazy val additionalJavaSourceDirectories = Def.setting { + getAdditionalSourceDirectories( + (Compile / sourceDirectory).value, + JavaSourceDirectories) + } + + lazy val additionalSourceDirectories = Def.setting { + additionalScalaSourceDirectories.value ++ additionalJavaSourceDirectories.value + } + + private lazy val additionalScalaTestSourceDirectories = Def.setting { + getAdditionalSourceDirectories( + (Test / sourceDirectory).value, + ScalaTestSourceDirectories) + } + + private lazy val additionalJavaTestSourceDirectories = Def.setting { + getAdditionalSourceDirectories( + (Test / sourceDirectory).value, + JavaTestSourceDirectories) + } + + lazy val additionalTestSourceDirectories = Def.setting { + additionalScalaTestSourceDirectories.value ++ additionalJavaTestSourceDirectories.value + } + + private def getAdditionalSourceDirectoryNames(language: String, isTest: Boolean): Seq[String] = { + for { + version <- supportedJavaLTSVersions if version.toInt <= JdkOptions.JavaVersion.majorVersion + } yield { + if (isTest) { + s"$language-jdk${version}-only" Review Comment: I notice you are using string interpolation, why not use `suffix` argument rather than a boolean, we could gain beautiful code. ########## project/JdkOptions.scala: ########## @@ -27,6 +27,19 @@ object JdkOptions extends AutoPlugin { lazy val specificationVersion: String = sys.props("java.specification.version") + object JavaVersion { + val majorVersion: Int = { Review Comment: I am fine with that, but it will be great if we have a better solution for this. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
