Add isCircularReference to properties #4553
Merged
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
wing328
reviewed
Nov 28, 2019
| @@ -347,6 +347,7 @@ public int compare(CodegenModel cm1, CodegenModel cm2) { | |||
| }); | |||
| } | |||
| } | |||
| setCircularReferences(allModels); | |||
There was a problem hiding this comment.
Can we add this to the default codegen instead as users would expect isCircularReference with a valid value (true, false) for all generators?
There was a problem hiding this comment.
@wing328 that was my intention, that's why I added it to updateAllModels in DefaultCodegen. Somehow this did got not triggered in Elm, so I thought I'd probably overridden it. Is updateAllModels the correct place to put this?
There was a problem hiding this comment.
Can you please move it to updateAllModels and I'll run some tests later to troubleshoot?
There was a problem hiding this comment.
@wing328 I already put it in updateAllModels. Do I need to remove it from the Elm codegen?
eriktim
force-pushed
the
circular-references
branch
from
395cffe
to
4190c50
Compare
5 tasks
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.
As a preparation to a major refactoring of the Elm generator I would like to add the ability of detecting circular references. Support for self referring types was already in place, but this PR also detects types that indirectly refer back to themselves. Every property that indirectly refers back to itself gets marked with
isCircularReference.PR checklist
./bin/(or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run./bin/{LANG}-petstore.sh,./bin/openapi3/{LANG}-petstore.shif updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).master,4.3.x,5.0.x. Default:master.cc @wing328 @jimschubert @cbornet @ackintosh @jmini @etherealjoy