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

Allow to retrieve schema type parameters full name #3500

Merged
merged 5 commits into from
Feb 9, 2024

Conversation

yakivy
Copy link
Contributor

@yakivy yakivy commented Feb 5, 2024

@kciesielski
Copy link
Member

One question regarding the issue report:

Specifically Value[A.B] and Value[B.B] produce the same Schema.SName that causes a conflict in the docs components

Wouldn't that produce Value_B1 and Value_B2?
Besides, I would expect this PR to require changes in tests like this one
https://github.com/softwaremill/tapir/blob/master/docs/openapi-docs/src/test/scalajvm/sttp/tapir/docs/openapi/VerifyYamlTest.scala#L254

but they still pass, so maybe I don't understand well what really is affected?

@yakivy
Copy link
Contributor Author

yakivy commented Feb 8, 2024

hey @kciesielski , this PR only puts full type name into the Schema.SName.typeParameterShortNames so you can use them in the custom schema mappers, all other logic is kept backward compatible. For example default schema name mapper still takes only last symbol name:
https://github.com/softwaremill/tapir/pull/3500/files#diff-8a362583486937e726273911ac38096a6d5037ac832a634d021c55297ce6bfacR8

@kciesielski
Copy link
Member

I see, thank you for clarifying!

@adamw
Copy link
Member

adamw commented Feb 8, 2024

Thanks! :) One small question - maybe it would make sense to add a test, which shows that you can customise the schema naming, fixing the original problem from #3463?

@adamw adamw merged commit fee45e2 into softwaremill:master Feb 9, 2024
23 checks passed
@adamw
Copy link
Member

adamw commented Feb 9, 2024

Thanks :)

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.

3 participants