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: Refactor SwaggerGen to support multiple values with the same path #145

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dwertent
Copy link

This PR offers an alternative solution to the issue addressed in this PR. Rather than modifying the API, we've adjusted the behavior of the Swagger generation to ensure the value is no longer ignored.

Signed-off-by: dwertent <david.wertenteil@kaleido.io>
pkg/ffapi/openapi3.go Outdated Show resolved Hide resolved
Signed-off-by: dwertent <david.wertenteil@kaleido.io>
Copy link
Contributor

@EnriqueL8 EnriqueL8 left a comment

Choose a reason for hiding this comment

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

Just want to understand where normalising the path makes sense

@@ -116,7 +116,7 @@ func (sg *SwaggerGen) getPathItem(doc *openapi3.T, path string) *openapi3.PathIt
if doc.Paths == nil {
doc.Paths = &openapi3.Paths{}
}
pi := doc.Paths.Find(path)
pi := doc.Paths.Value(path)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just being careful with this, as it would affect all the swagger documents generated from now on

Is there a scenario where normalising does make sense? I tried to find the history in the kin-openapi repo and it seems that it was there from the original commit

Copy link
Author

Choose a reason for hiding this comment

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

Ok, we can wait for a response here.

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.

2 participants