Preserve map ordering when transforming #13
Conversation
diamondburned
commented
Aug 12, 2024
diamondburned
commented
This vastly improves the Go test output. The benchmark is also added for the next commit which changes the behavior of this library and may potentially be marginally slower.
|
Benchmark results: ( |
This commit changes both JSONToYAML and YAMLToJSON to preserve the map key ordering when the document is transformed. This is done by reimplementing the library to use YAML v3's yaml.Node API.
|
Some of this work was borrowed from @silasdavis and @skipor from PR ghodss/yaml#62. |
This commit adds the `OrderedPropertyKeys` method to the
`openapi3.Schema`:
OrderedPropertyKeys returns the keys of the properties in the order
they were defined. This is useful for generating code that needs to
iterate over the properties in a consistent order. If the keys could
not be extracted for some reason, then this method automatically
sorts the keys to be deterministic.
This is done via a temporary fork of the YAML-to-JSON transformation library.
It will not be ready until invopop/yaml#13 is merged.
This commit adds the `PropertyKeys` property to the type `openapi3.Schema` which contains the keys of the `Properties` map in the order that they appear in the original YAML file. This is useful to guarantee deterministic code generation. This is done via a temporary fork of the YAML-to-JSON transformation library. It will not be ready until invopop/yaml#13 is merged.
This commit adds the `PropertyKeys` property to the type `openapi3.Schema` which contains the keys of the `Properties` map in the order that they appear in the original YAML file. This is useful to guarantee deterministic code generation. This is done via a temporary fork of the YAML-to-JSON transformation library. It will not be ready until invopop/yaml#13 is merged.
|
What is the status @samlown I understand you guys patched the security problem and there is that. Do you plan to push new features? I am interested in this and the date parsing problem reported the other day as well. Cheers! |
|
@lzap I honestly didn't have this PR on my radar. We actively use this package in other projects, so our plan is to maintain as much as possible, we're just super-low on time. It looks like @diamondburned did a really good job here, so I'll get it on my Todos. |
|
So this one get a bit more complicated. The go-yaml project on which this is based has been archived 😞 Seems like the best option would be to migrate away to a different solution rather than update to something that is no longer active. |
|
I do not think there is need to migrate off this library, k8s folks already forked it and are fully committed to do security maintenance (no new features): https://github.com/kubernetes-sigs/yaml/tree/master/goyaml.v3 that should be pretty straightforward replacement. |
|
Nice. I was actually thinking of doing something similar and stripping out the bits we don't need. However, it looks like the underlying project there is a direct fork of the same ghodss/yaml package this one is forked from! I suspect the Kubernetes team are better at maintaining this than me, it could be a much better option. |
Oh I missed that :-D Well whatever happens, I am very interested in preserving map ordering, it is a massive pain. @diamondburned would you mind opening this PR at k8s to see how responsive they are? Chances are they might be opinionated on this. |
3 participants
