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

(#51) add "omitempty" option to nilable fields #73

Merged
merged 1 commit into from Jul 14, 2021

Conversation

blukai
Copy link
Contributor

@blukai blukai commented Jun 2, 2021

I faced the problem that is described in #51 and after short investigation came up with this solution.
please let me know if tests needs to be added, etc.

closes #51

@Yamashou
Copy link
Owner

Yamashou commented Jun 3, 2021

please tell me the result of generating code. I want to see that.

@blukai
Copy link
Contributor Author

blukai commented Jun 3, 2021

example repo with two branches with-omitempty and without-omitempty:
https://github.com/blukai/gqlgenc-omitempty-example

here's the diff blukai/gqlgenc-omitempty-example@cec3d5a

@Yamashou
Copy link
Owner

Yamashou commented Jun 9, 2021

LGTM!

I will merge as soon as I can confirm that the generated code is correct!

@mlaflamm
Copy link
Contributor

This is exactly what I need. I would love to see this merged.

@mlaflamm
Copy link
Contributor

Shouldn't omitempty only present on input structs?

@Yamashou
Copy link
Owner

Shouldn't omitempty only present on input structs?

@mlaflamm
So sorry, I have so many work. your opinion is a correct. Should I fix that ?

@Yamashou
Copy link
Owner

@mlaflamm
your saying is other PR. please waiting soon!

@Yamashou Yamashou merged commit a37411a into Yamashou:master Jul 14, 2021
@mlaflamm
Copy link
Contributor

mlaflamm commented Jul 14, 2021

Thank you for merging this.

I don't think that having omitempty everywhere has any negative impacts other than potentially mislead someone reading the generated code as thinking this is useful for non input structs. IMHO, having omitempty only on input is a low priority change.

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.

Nullable fields should generate JSON structs with omitempty option
3 participants