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

go: add a page about YAML marshalling #73

Merged
merged 4 commits into from
Sep 19, 2019
Merged

Conversation

kopiczko
Copy link
Contributor

@kopiczko kopiczko commented Sep 18, 2019

According to the discussion in SIG operator.

@kopiczko kopiczko self-assigned this Sep 18, 2019
go/yaml_marshalling.md Outdated Show resolved Hide resolved
@kopiczko kopiczko requested a review from a team September 18, 2019 10:39
Copy link

@sslavic sslavic left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@rossf7 rossf7 left a comment

Choose a reason for hiding this comment

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

LGTM. The simplification of the CRDs is nice since we need to manage lots of them.

> JSON methods MarshalJSON and UnmarshalJSON unlike go-yaml.

There is a good explanation for doing so written down here:
http://web.archive.org/web/20190603050330/http://ghodss.com/2014/the-right-way-to-handle-yaml-in-golang/.
Copy link
Contributor

Choose a reason for hiding this comment

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

<3

There is a good explanation for doing so written down here:
http://web.archive.org/web/20190603050330/http://ghodss.com/2014/the-right-way-to-handle-yaml-in-golang/.

This implies that we do not tag Go structs with `yaml` tags. Only `json` tags
Copy link
Contributor

Choose a reason for hiding this comment

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

I would highlight this phrase.

@kopiczko kopiczko merged commit 17cb2e8 into master Sep 19, 2019
@kopiczko kopiczko deleted the go-yaml-marshalling branch September 19, 2019 17:15
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.

6 participants