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

Switch back to official YAML repo #28

Closed
joeduffy opened this issue Dec 3, 2016 · 0 comments
Closed

Switch back to official YAML repo #28

joeduffy opened this issue Dec 3, 2016 · 0 comments
Labels
area/tooling kind/bug Some behavior is incorrect or out of spec

Comments

@joeduffy
Copy link
Member

joeduffy commented Dec 3, 2016

The ghodss/yaml package contains a bug that I tripped on when converting our AST data structures over to using pointers in their maps instead of values. (This was required to avoid making copies of nodes which caused problems elsewhere.) Unfortunately, this bug seems to be known: there is an outstanding PR ghodss/yaml#7 and at least one open issue pertaining to this.

For now, I have forked the library to joeduffy/yaml, and fixed the bug. This is obviously not a great long-term strategy. I also didn't work to narrow down the root cause to a minimal repo, since I really don't understand much of why that reflection fu is in there to begin with.

I was tempted to navigate away from this library and towards go-yaml/yaml, however, then I read the original motivation for ghodss's approach: http://ghodss.com/2014/the-right-way-to-handle-yaml-in-golang/. I agree with the overall spirit (though worry about the performance). And even a 1 minute thought exercise of the annotation bloat that would result made my stomach ache.

So for now, I've gone the quick-and-dirty route. We likely need to revisit our choices here anyway as soon as we tackle #4, so I didn't want to invest too much time right now.

@joeduffy joeduffy added area/tooling kind/bug Some behavior is incorrect or out of spec labels Dec 3, 2016
joeduffy added a commit that referenced this issue Dec 3, 2016
See #28 for details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tooling kind/bug Some behavior is incorrect or out of spec
Projects
None yet
Development

No branches or pull requests

1 participant