-
Notifications
You must be signed in to change notification settings - Fork 77
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
feat: Add support for flexible defaults encoding #1652
feat: Add support for flexible defaults encoding #1652
Conversation
modules/core/src/smithy4s/codecs/FieldRenderPredicateCompiler.scala
Outdated
Show resolved
Hide resolved
modules/core/src/smithy4s/codecs/FieldRenderPredicateCompiler.scala
Outdated
Show resolved
Hide resolved
modules/core/src/smithy4s/codecs/FieldRenderPredicateCompiler.scala
Outdated
Show resolved
Hide resolved
modules/core/src/smithy4s/codecs/FieldRenderPredicateCompiler.scala
Outdated
Show resolved
Hide resolved
I am not happy about the implementation of |
modules/core/src/smithy4s/codecs/FieldRenderPredicateCompiler.scala
Outdated
Show resolved
Hide resolved
modules/core/src/smithy4s/codecs/FieldRenderPredicateCompiler.scala
Outdated
Show resolved
Hide resolved
modules/core/src/smithy4s/codecs/FieldRenderPredicateCompiler.scala
Outdated
Show resolved
Hide resolved
modules/core/src/smithy4s/codecs/FieldRenderPredicateCompiler.scala
Outdated
Show resolved
Hide resolved
modules/core/src/smithy4s/codecs/FieldRenderPredicateCompiler.scala
Outdated
Show resolved
Hide resolved
modules/core/src/smithy4s/codecs/FieldRenderPredicateCompiler.scala
Outdated
Show resolved
Hide resolved
modules/core/src/smithy4s/codecs/FieldRenderPredicateCompiler.scala
Outdated
Show resolved
Hide resolved
A good start 👍 |
@kubukoz wanna have a look? |
61e124e
to
288af6a
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.
@Baccata please have a final look too
Good work. Just a couple nitpicks (and also this : #1652 (comment)) |
PR Checklist (not all items are relevant to all PRs)
Added bootstrapped code + smoke tests (when the rendering logic is modified)N/AAdded build-plugins integration tests (when reflection loading is required at codegen-time)N/AAdded alloy compliance tests (when simpleRestJson protocol behaviour is expanded/updated)N/AUpdated dynamic module to match generated-code behaviourN/ARefers to #1398