-
Notifications
You must be signed in to change notification settings - Fork 3
Sandbox/dashboard v1 + v2 #722
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
base: main
Are you sure you want to change the base?
Conversation
Note: the diff show code changes that would be introduced by this PR to the output of the
config/foundation_sdk.dev.yaml |
ea02e74
to
363627f
Compare
dfc3e3c
to
e65dbab
Compare
5cfc397
to
7ca48a9
Compare
9755baa
to
a38ecb6
Compare
6a6cc7f
to
edf7688
Compare
…s/v2alpha1/dashboard_spec.cue
833d68f
to
a8eaecc
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
😢 zizmor failed with exit code 14. Expand for full output
|
@@ -158,6 +158,8 @@ func (language *Language) CompilerPasses() compiler.Passes { | |||
&compiler.FlattenDisjunctions{}, | |||
&compiler.DisjunctionInferMapping{}, | |||
&compiler.UndiscriminatedDisjunctionToAny{}, | |||
&compiler.DisjunctionToType{}, | |||
&compiler.RemoveIntersections{}, |
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.
This is making my eyebrows go up 😆
Why are these compiler passes needed? I'm especially worried about the DisjunctionToType
one: it shouldn't be needed and using it will actually introduce a bunch of completely avoidable BC breaks in the foundation SDK
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 added because it "fixes" the converter in PHP. One of the affected schemas is Expr, but I think that is because this veneer.
The problem is that without DisjunctionToType
, the disjunction appears in the guard
block (here), instead of the BuilderDisjunction (here), making it more complex.
The second compiler pass is to remove aliases (like Java).
Maybe we have any bug in the veneer? It also applies disjunction_to_type
, but at the beginning.
Relates to #653
Spiritual successor of #664
Todo: