Skip to content
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

chore(external docs): Make better use of Cue definitions #4493

Merged
merged 14 commits into from Oct 10, 2020

Conversation

binarylogic
Copy link
Contributor

@binarylogic binarylogic commented Oct 10, 2020

As recommended by a Cue maintainer, we should be making judicious use of definitions:

  1. Definitions are implicitly closed, making validation more strict.
  2. Definitions provide better error messages.
  3. Definitions are significantly faster.

Signed-off-by: binarylogic <bjohnson@binarylogic.com>
Signed-off-by: binarylogic <bjohnson@binarylogic.com>
@binarylogic binarylogic changed the title Cue composition chore(external docs): Make better use of Cue definitions Oct 10, 2020
@binarylogic
Copy link
Contributor Author

@MOZGIII this appears to be the correct way to make the validation stricter.

Signed-off-by: binarylogic <bjohnson@binarylogic.com>
@MOZGIII
Copy link
Contributor

MOZGIII commented Oct 10, 2020

@MOZGIII this appears to be the correct way to make the validation stricter.

Interesting! I don't see how it addressed my concerns with the types for the log output:

https://github.com/timberio/vector/blob/9272eca34837f4bb79c0ca88bdf73da090748b99/docs/reference.cue#L353-L365

That code in this branch is the same as before. Is this intended?


I wonder what cue maintainers say about the computation time increase. Can they look into our use case and provide a better solution, something that would exclusively allow only a single field? Or does this do it already?

Signed-off-by: binarylogic <bjohnson@binarylogic.com>
Signed-off-by: binarylogic <bjohnson@binarylogic.com>
@@ -351,16 +353,16 @@ _values: {
relevant_when?: string
required: bool
type: {
"*": {}
"[string]"?: {
{"*": {}} |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will end up with a big oops unless closed. The evaluation will assume all types satisfy the values here, and rightfully so, since they can be expanded to satisfy the value, as they're not closed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean exactly?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it works - nevermind :D

Signed-off-by: binarylogic <bjohnson@binarylogic.com>
Signed-off-by: binarylogic <bjohnson@binarylogic.com>
Signed-off-by: binarylogic <bjohnson@binarylogic.com>
Signed-off-by: binarylogic <bjohnson@binarylogic.com>
Signed-off-by: binarylogic <bjohnson@binarylogic.com>
Signed-off-by: binarylogic <bjohnson@binarylogic.com>
Signed-off-by: binarylogic <bjohnson@binarylogic.com>
Signed-off-by: binarylogic <bjohnson@binarylogic.com>
Signed-off-by: binarylogic <bjohnson@binarylogic.com>
@binarylogic binarylogic merged commit 32051bd into master Oct 10, 2020
@binarylogic binarylogic deleted the cue-composition branch October 10, 2020 20:16
mengesb pushed a commit to jacobbraaten/vector that referenced this pull request Dec 9, 2020
…v#4493)

Signed-off-by: Brian Menges <brian.menges@anaplan.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants