Ensure that imports are not removed in generated code #406
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
Makes it such that the "ptimports.Process" call on generated code does not add or remove imports. Fixes an issue where generated code would not include imports that referenced packages where the last component of the package path was of the form "v[0-9]+". Fixes #405
Generate changelog in
|
bmoylan
approved these changes
Jan 27, 2023
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 6 of 6 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @tabboud)
bulldozer-bot bot
pushed a commit
that referenced
this pull request
Feb 1, 2023
…#408) Reverts the conjure-go behavior change introduced in v6.36.0 to match previous versions: certain named imports generated by Conjure code will only be generated if the Conjure generator is run twice, but once this is done, the output of `conjure.Generate` and `conjure.GenerateOutputFiles` will match, which prevents verification issues.
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.
Makes it such that the "ptimports.Process" call that is called for individual generated code files does not add or remove imports and then adds a step where "ptimports.Process" is called on all generated code files after they are written. Fixes an issue where generated code would not include imports that referenced packages where the last component of the package path was of the form "v[0-9]+".
Fixes #405
Before this PR
See #405 for details, but before this PR, generating code for Conjure definitions that have packages where the last component is of the form
v[0-9]+(for example, "com.palantir.foo.v2") would not generate the import statement needed to compile (so the first run of the generator would produce code that didn't compile). Running the generator a second time would add the expected imports.After this PR
==COMMIT_MSG==
Fixes an issue where generated code would not include imports that referenced packages where the last component of the package path was of the form "v[0-9]+".
==COMMIT_MSG==
Possible downsides?
This fix adds a step where, after all the generated files are written, they are all formatted and written again. This process adds computational overhead on the order of the number of files written, but I don't think it should be a major concern as the operation is not too expensive and generating the files is not a hot path (it's an operation that is expected to take a decent amount of time and is run in a one-off manner in workflows).
This change is