-
Notifications
You must be signed in to change notification settings - Fork 1k
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
upgrade to yaml.v3 #1148
base: master
Are you sure you want to change the base?
upgrade to yaml.v3 #1148
Conversation
The main nice fix here is that maps unmarshal into `map[string]any` instead of `map[any]any`, so it cleans things up a bit.
Since yaml.v3 doesn't automatically convert yes to bool now, for backwards compat
case bool: | ||
return x, true | ||
case string: | ||
switch x { |
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.
Does this work for Yes
?
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.
probably not, let me see what yaml.v2
did for this
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.
ah this is what they are doing in v3 if you unmarshal into a Bool:
So we can probably copy that.
This is what v2 did:
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.
although if you look above this file at the existing config.GetBool
it only allowed the lowercase versions. So we might want to match that for backwards compat. This new config.AsBool is only used for allow_list entries.
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.
Existing in v1.9.5: https://github.com/slackhq/nebula/blob/v1.9.5/config/config.go#L257-L262
The main nice fix here is that maps unmarshal into
map[string]any
instead of
map[any]any
, so it cleans things up a bit.The only gotcha I see for our uses is that the Yaml 1.1 bool types like
yes
won't automatically be set as a bool inside ofmap[string]any
. So I added a little helperconfig.AsBool
to keep backwards compat there.