Make environment variables access platform-specific #9282
Conversation
|
|
|
|
||
| private[zio] trait SystemPlatformSpecific { self: System.type => | ||
|
|
||
| trait SystemLivePlatformSpecific extends System { |
There was a problem hiding this comment.
Let's make this private[zio] as well
|
|
||
| private[zio] trait SystemPlatformSpecific { self: System.type => | ||
|
|
||
| trait SystemLivePlatformSpecific extends System { |
|
I'm considering whether implementing the entire Additionally, regarding the system properties and Scala.js System.scala implementation // Excerpt from Scala.js System.scala
def getProperties(): ju.Properties = SystemProperties.getProperties()
def lineSeparator(): String = "\n"
// Other methods...These methods are maintained within the Scala.js standard library, providing compatibility with the Java API. Since they are already implemented and maintained by Scala.js, perhaps we can defer to these existing implementations instead of re-implementing them in our platform-specific code. What do you think about tailoring the platform-specific code specifically towards environment variables and relying on the existing Scala.js implementations for system properties and line separators? This approach might help us avoid unnecessary duplication and leverage the maintenance work already done by the Scala.js team. I'm open to your thoughts on this. Do you think it's better to focus the platform-specific code solely on environment variables, or should we keep the complete Thanks! |
|
Something like this: diff --git a/core/shared/src/main/scala/zio/System.scala b/core/shared/src/main/scala/zio/System.scala
--- a/core/shared/src/main/scala/zio/System.scala (revision f092e25d1bfa01eb8dae4dc656ff8e623894ee3b)
+++ b/core/shared/src/main/scala/zio/System.scala (date 1730986534534)
@@ -101,7 +101,7 @@
}
}
-object System extends Serializable {
+object System extends SystemPlatformSpecific {
val tag: Tag[System] = Tag[System]
@@ -140,7 +140,7 @@
@transient override val unsafe: UnsafeAPI =
new UnsafeAPI {
override def env(variable: String)(implicit unsafe: Unsafe): Option[String] =
- Option(JSystem.getenv(variable))
+ environmentProvider.env(variable)
override def envOrElse(variable: String, alt: => String)(implicit unsafe: Unsafe): String =
envOrElseWith(variable, alt)(env)
@@ -148,9 +148,8 @@
override def envOrOption(variable: String, alt: => Option[String])(implicit unsafe: Unsafe): Option[String] =
envOrOptionWith(variable, alt)(env)
- @nowarn("msg=JavaConverters")
override def envs()(implicit unsafe: Unsafe): Map[String, String] =
- JSystem.getenv.asScala.toMap
+ environmentProvider.envs
override def lineSeparator()(implicit unsafe: Unsafe): String =
JSystem.lineSeparator
@@ -172,6 +171,11 @@
}
}
+ private[zio] trait EnvironmentProvider {
+ def env(variable: String): Option[String]
+ def envs: Map[String, String]
+ }
+
private[zio] def envOrElseWith(variable: String, alt: => String)(env: String => Option[String]): String =
env(variable).getOrElse(alt)
|
|
@mhriemers yeah I think the approach you proposed makes sense. It should also be bin-compatible so let's go with it 👍 |
|
@kyri-petrou I prepared the commit yesterday, including your review comments. |
|
💵 To receive payouts, sign up on Algora, link your Github account and connect with Stripe. |
* Make environment variables access platform-specific * More environment variable specific API
Fixes #9279
/claim #9279