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

fix: seesharp: Using dependency management / partOfProperty to render optional types with question mark #1037

Closed
wants to merge 11 commits into from

Conversation

zaytsevand
Copy link
Contributor

Description

  • Using ConstrainedObjectPropertyModel to get info on if the property is required

Related issue(s)
Fixes #941

@zaytsevand zaytsevand changed the title fix: CSharp: Using dependency management / partOfProperty to render optional types with question mark fix: seesharp: Using dependency management / partOfProperty to render optional types with question mark Dec 6, 2022
Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

Before this PR, type and name were interchangeable because they contained the same value. Now they no longer do that, and we were not strict in the past when what was used 😅

Other then that you have some linting errors 🙂

@zaytsevand
Copy link
Contributor Author

zaytsevand commented Dec 7, 2022

Updated. Thanks, I missed that over all the changes!
Btw, from my understanding, all the hassle with custom Converter classes is there because of enums. Am I getting it correctly?
If so, do you mind shortening a lot of what is generated by using another technique to customize output?
Code example (Newtonsoft.Json): https://dotnetfiddle.net/365DN6
System.Text.Json support is not yet there, unfortunately: dotnet/runtime#74385 (comment)

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 8, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jonaslagoni
Copy link
Member

Btw, from my understanding, all the hassle with custom Converter classes is there because of enums. Am I getting it correctly?

Do you mean the to model and GetValue methods?

They are there because the core models are not meant to depend on any outside library, that is what the presets are for 🙂

You could definitely argue that the NewtonsoftSerializerPreset should add this annotation to the enums 👍

Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

@zaytsevand it seems you have merged the dependency manager changes into the branch again. I have a feeling you have tainted your local next branch as you merged it into your branch here: d2a2885

@zaytsevand zaytsevand closed this Dec 8, 2022
@zaytsevand zaytsevand deleted the asyncapi/modelina/next branch December 8, 2022 20:11
@zaytsevand zaytsevand restored the asyncapi/modelina/next branch December 8, 2022 20:13
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