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

Omit empty Creative>CreativeExtensions and InLine>Extensions #28

Merged
merged 2 commits into from
Apr 17, 2018

Conversation

jussi-kalliokoski
Copy link
Contributor

Fixes #27. I don't really like this solution, but given how go marshals nested tags I couldn't come up with a better solution.

@rs
Copy link
Owner

rs commented Apr 17, 2018

@kalbasit please review

Copy link
Contributor

@kalbasit kalbasit left a comment

Choose a reason for hiding this comment

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

@rs LGTM

@rs rs merged commit e5c78ea into rs:master Apr 17, 2018
@louie432843
Copy link

@jussi-kalliokoski out of curiosity have you ever run across another way to deal with how go marshals nested tags other than using pointers to structs?

@jussi-kalliokoski jussi-kalliokoski deleted the omit_empty_creative_extensions branch March 3, 2022 09:35
@jussi-kalliokoski
Copy link
Contributor Author

@louie432843 unfortunately not, the XML marshal in go has remained relatively unchanged for a long time and I suspect the reason is that most use cases for go + XML are not very performance-sensitive (with a few exceptions, such as VAST ☹️ ). So the language probably considers it an OK tradeoff to take the performance hit for correctness (or to use template structs where you predefine all your would-be-pointers in structs so as to avoid the allocations, similar to how protobuf is often optimized where necessary) rather than to make improvements (breaking or otherwise) to the standard API.

@louie432843
Copy link

Thanks for the reply!

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.

omitempty not working for Creative>CreativeExtensions and InLine>Extensions
4 participants