-
Notifications
You must be signed in to change notification settings - Fork 46
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
🐛 Implement sorting on structs when marshaling #431
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just thinking out loud here, but what do we think about doing this for JSON while we do this?
I don't think encoding/json does sorting by default so it may be worth it.
return d, nil | ||
} | ||
|
||
func (d Dep) cmpLess(other Dep) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I accidentally left it in before committing. I was doing some exploration on both JSON and YAML. Hopefully the "Implemented JSON support" commit elucidates it
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
8616c58
to
275f0f2
Compare
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
Fixes #418
demo-output.yaml
's diff looks really ugly, but all this PR does is shuffle some stuff around.With this, we may not the
sed
andyq
stuff in this workflow anymore:analyzer-lsp/.github/workflows/demo-testing.yml
Lines 20 to 25 in 740142e