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

Add a toggle to disable "omitempty" #282

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ptodev
Copy link

@ptodev ptodev commented Sep 19, 2024

Adding a toggle to not include the "omitempty" tag from generated struct fields.

This could be useful if you're migrating an already existing codebase which doesn't use "omitempty" to go-jsonschema and want to minimise the number of changes.

The codebase which I'm working on uses mapstructure and almost never uses omitempty. Instead, it has decoder hooks to manipulate the decoding in various ways. To be honest, I think having the omitempty might not change anything about the decoding process in my particular case. But I think having such a toggle could help make sure there are no unintended consequences from having an omitempty.

I don't think most people would want to have a DisableOmitempty = true though. If you use json or yaml decoding, the validation functions which go-jsonschema generates wouldn't work correctly if omitempty is missing, since the validator wouldn't know that a non-required property is missing and it wouldn't be able to set it to its default value. My change is probably only useful for certain users of mapstructure.

An alternative way to do this change could be to add it to goJSONSchema setting so that we can override it per property, instead of setting it globally. Maybe we should have both types of toggles - a global, and a property-specific one?

@omissis
Copy link
Owner

omissis commented Nov 16, 2024

@ptodev I think this is ok, can you please fix the linting and deal with the todo, I will merge it then. also take this out of draft once it's ready. thank you!

@ptodev ptodev marked this pull request as ready for review November 18, 2024 18:04
Copy link

codecov bot commented Nov 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.19%. Comparing base (d963216) to head (b7cea37).
Report is 96 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #282      +/-   ##
==========================================
- Coverage   76.58%   73.19%   -3.40%     
==========================================
  Files          24       43      +19     
  Lines        1892     2839     +947     
==========================================
+ Hits         1449     2078     +629     
- Misses        354      588     +234     
- Partials       89      173      +84     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

Copy link
Author

Choose a reason for hiding this comment

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

The changes in the go.mod files are from running go mod tidy.

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