Add LocalTime & Maybe SDK Functions #572
Conversation
- MaybeSDK.map2
- MaybeSDK.map3
- MaybeSDK.map4
- MaybeSDK.hasValue
- LocalTimeSDK.diffInHours
- LocalTimeSDK.diffInMinutes
| (ctx: NativeContext) => (f: RT.Function, maybeRaw1: RT.ConstructorResult, maybeRaw2: RT.ConstructorResult) => | ||
| { | ||
| val out = for { | ||
| elem1 <- eitherToOption(maybeRaw1) |
There was a problem hiding this comment.
So i was confused by the use of eitherToOption but it turns out that is a a poorly named function. @edwardpeters can you please add appropriate docs for this function in another PR and rename it to something that actually reflects what is happening?
There was a problem hiding this comment.
There's part of this implementation that feels strange, but I'm a bit swamped to review this right now... (@michelchan , @edwardpeters , or @Paul-Folbrecht ) can any of you take a look at this?
There was a problem hiding this comment.
Agreed. The comments say ConstructorResult is used for Maybes and Results, and neither are an Either. So this should at least be renamed.
There was a problem hiding this comment.
Yeah, looks like those got mangled when they were copied from the similar Result/Either helpers. Have a PR in to fix this, currently building.
morphir/runtime/test/jvm/src/org/finos/morphir/runtime/EvaluatorMDMTests.scala
Show resolved
Hide resolved
morphir/runtime/test/jvm/src/org/finos/morphir/runtime/EvaluatorMDMTests.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I'm good with this, but would like to see #575 addressed as mentioned in the comments
| (ctx: NativeContext) => (f: RT.Function, maybeRaw1: RT.ConstructorResult, maybeRaw2: RT.ConstructorResult) => | ||
| { | ||
| val out = for { | ||
| elem1 <- eitherToOption(maybeRaw1) |