-
Notifications
You must be signed in to change notification settings - Fork 22
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
Add NewNames and Value.NormalizeNames #9
base: master
Are you sure you want to change the base?
Conversation
The NewNames function produces a map of canonical names from a Go struct. It can be passed to Value.NormalizeNames to normalizes case-insensitive matches to a JSON object name to the canonical name.
GitHub just pinged me about this. Do you still need a review here? Sitting since September 9th makes me think this wasn't needed? |
It's still helpful so that we can auto-format the ACL files to use consistent names. It's not critical. |
Talked to @josharian; he's taking this over from me. |
So far, I've looked just at the new API, not the implementation. I think I understand the goal, but I'm not sure the API quite matches. Consider: type LL struct {
S string
Next *LL `json:"ref"`
} I don't think there's a way to write a It seems like we want a mapping per type, but there's nothing good to use as a key. Maybe instead the "Names" type should be a single Maybe instead the "Names" type should embrace this shortcoming and be a single flat map. And if you need to rename keys differently based on context, oh well? |
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.
Using the official "request changes" button to get GitHub to stop nagging at me that it's my turn on this PR. :)
The NewNames function produces a map of canonical names from a Go struct.
It can be passed to Value.NormalizeNames to normalizes case-insensitive
matches to a JSON object name to the canonical name.