Skip to content

Conversation

dwertent
Copy link
Contributor

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>
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

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
Contributor 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Circling back to this issue, I tested it (back in the day) and it worked.

@EnriqueL8
Copy link
Contributor

@dwertent Could you take a look at this one so we can close it?

Signed-off-by: David Wertenteil <david.wertenteil@kaleido.io>
@dwertent dwertent requested a review from a team as a code owner June 6, 2025 14:52
@dwertent dwertent requested a review from EnriqueL8 June 6, 2025 15:19
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.

Looks good - we need to be careful when pulling this into other repos and make sure the changes are correct

@EnriqueL8 EnriqueL8 changed the title chore: Refactor SwaggerGen to support multiple values with the same path fix: change SwaggerGen to support multiple values with the same path Jul 2, 2025
@EnriqueL8 EnriqueL8 merged commit 1bb155a into hyperledger:main Jul 3, 2025
5 checks passed
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