[DO NOT MERGE] Illustrate example quality improvement #3134
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mikhailshilkov
added a commit
that referenced
this pull request
Mar 11, 2024
### Problem Program generation (`x.GenerateProgram` functions for each language) have side effects since they change the `program` object passed to them in-place. Currently, we pass the same `program` object through the entire pipleline of example generation for all languages. This means, that all languages except nodejs (which is the first in the list) is affected by previous languages in subtle ways. Mostly, this seems to break schema type information in hard-to-understand ways. See the list of issues and a diff link below for examples. ### Change This PR moves the invocation of `hcl2.BindProgram` inside the language loop, which prevents us from sharing the `program`. This should eliminate the cross-language effects. Entirely unexpectedly to me, this leads to a substantial performance _gain_ not penalty: the schema generation time reduced from 110s to 13s on my laptop! The second code change you see here is extending the ability to write examples to disk from HCL-only to any language. (see below) ### Validation I logged all examples in all languages before and after the change, and pushed them to [this PR](#3134). This way it's easier to validate the results. 700+ python, 200+ go, and a number of csharp examples are pure improvements with better type instantiations. The only seeming regression are C# changes from `new[] {}` to `new() {}` where the old option looks correct. I guess that's something that worked by luck then? ### Issues Fixed Fix #2608 Fix #2575 Fix #2084 Fix #2083 Fix #2275 Fix #2047 Fix #1479 Fix #2827 (the example it shows is fixed)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.