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

feat: #2114 - add required field management in csharp newtonsoft serializer #2117

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

fr-th
Copy link

@fr-th fr-th commented Oct 29, 2024

Description

When generating c# model with newtonsoft serialization, an option now allows to prevent deserialization if required fields are missing

Related Issue

#2114

Checklist

  • The code follows the project's coding standards and is properly linted (npm run lint).
  • Tests have been added or updated to cover the changes.
  • Documentation has been updated to reflect the changes.
  • All tests pass successfully locally.(npm run test).

Additional Notes

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

Copy link

netlify bot commented Oct 29, 2024

Deploy Preview for modelina canceled.

Name Link
🔨 Latest commit 7bf5f90
🔍 Latest deploy log https://app.netlify.com/sites/modelina/deploys/6720bbc393667100087e9832

Copy link

sonarcloud bot commented Oct 29, 2024

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.

Do you want to continue with enforceRequired or make it part of the default rendering process?

All you need to make the changes on the next branch instead of master, but up to you 🙂

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.

** Still need to review 😄

@coveralls
Copy link

Pull Request Test Coverage Report for Build 11572166800

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.009%) to 92.305%

Totals Coverage Status
Change from base Build 11517599539: -0.009%
Covered Lines: 6004
Relevant Lines: 6335

💛 - Coveralls

@fr-th fr-th changed the base branch from master to next October 29, 2024 10:50
@fr-th
Copy link
Author

fr-th commented Oct 29, 2024

Ah damn, I missed the part for the 'next' branch sorry.
I'll adapt

@fr-th fr-th changed the base branch from next to master October 29, 2024 10:56
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