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

Revert #410 #557

Closed
stevvooe opened this issue Feb 6, 2017 · 5 comments
Closed

Revert #410 #557

stevvooe opened this issue Feb 6, 2017 · 5 comments
Milestone

Comments

@stevvooe
Copy link
Contributor

stevvooe commented Feb 6, 2017

#410

omitempty has nothing to do with REQUIRED vs OPTIONAL fields.

@stevvooe stevvooe added this to the v1.0.0 milestone Feb 6, 2017
@wking
Copy link
Contributor

wking commented Feb 6, 2017 via email

@philips
Copy link
Contributor

philips commented Feb 6, 2017

@stevvooe in #553 you are saying we should omitempty everywhere. So you would be ok with #410 if it was just doing omitempty but didn't mention optional/required?

@stevvooe
Copy link
Contributor Author

stevvooe commented Feb 6, 2017

@philips We should not emit fields that contain zero-values. It's wasteful and distracting.

There are certain exceptions, but, in general, use of omitempty should be the rule.

@wking
Copy link
Contributor

wking commented Feb 6, 2017 via email

@stevvooe
Copy link
Contributor Author

stevvooe commented Feb 8, 2017

I'm sorry but I misread #410.

For the most part, we should be using omitempty except in specific circumstances.

Closing this now. Apologies.

@stevvooe stevvooe closed this as completed Feb 8, 2017
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

No branches or pull requests

3 participants