-
Notifications
You must be signed in to change notification settings - Fork 7
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
Fix #383 - serializer now skips fields with null value secrets #386
Conversation
6035bba
to
2d7a0fe
Compare
552b9de
to
8a0364e
Compare
8a0364e
to
4ef02c6
Compare
dbda104
to
ba650a0
Compare
ba650a0
to
ff61ec8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some nitpicks but nothing major. Fix at your leisure and let's get core libs to 0.2.1.
@@ -405,6 +405,11 @@ DO NOT remove dependencies from `project.scala`, because they are necessary in b | |||
|
|||
## Troubleshooting | |||
|
|||
If you susspect the issue is related to serialization, try to skip the preview (dry run is known to be problematic): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo
@@ -52,7 +52,8 @@ trait EffectBesomModule extends BesomSyntax: | |||
config <- Config.forProject(runInfo.project) | |||
featureSupport <- FeatureSupport(monitor) | |||
_ <- logger.trace(s"Environment:\n${sys.env.toSeq.sortBy(_._1).map((k, v) => s"$k: $v").mkString("\n")}") | |||
_ <- logger.debug(s"Resolved feature support, spawning context and executing user program.") | |||
_ <- logger.debug(s"Resolved feature support: ${pprint(featureSupport)}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is pprint() returning a String or Unit? I am not sure if it just prints to stdout or converts into pretty printed string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it returns a Str and a toString is called, we already use it this way in the code so I did the same for consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, my mistake then
@@ -26,6 +25,9 @@ object EncoderTest: | |||
|
|||
case class PlainCaseClass(data: String, moreData: Int) derives Encoder | |||
case class OptionCaseClass(data: Option[String], moreData: Option[Int]) derives Encoder | |||
case class InputOptionalCaseClass(value: Output[Option[String]], data: Output[Option[Map[String, Output[String]]]]) | |||
derives Encoder, | |||
ArgsEncoder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's up with formatting here? 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scalafmt
had it's own idea ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lolwut
Fixes #383
The actual fix done by @lbialy