Conversation
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
Joerger
added
the
no-changelog
Joerger
force-pushed
the
joerger/enum-helper
branch
from
fb07aff to
c265007
Compare
| func (r *RequireMFAType) decode(val any) error { | ||
| err := decodeEnum(r, val, map[any]RequireMFAType{ | ||
| "": RequireMFAType_OFF, // default to off | ||
| false: RequireMFAType_OFF, | ||
| true: RequireMFAType_SESSION, | ||
| RequireMFATypeHardwareKeyString: RequireMFAType_SESSION_AND_HARDWARE_KEY, | ||
| RequireMFATypeHardwareKeyTouchString: RequireMFAType_HARDWARE_KEY_TOUCH, | ||
| RequireMFATypeHardwareKeyPINString: RequireMFAType_HARDWARE_KEY_PIN, | ||
| RequireMFATypeHardwareKeyTouchAndPINString: RequireMFAType_HARDWARE_KEY_TOUCH_AND_PIN, | ||
| }, RequireMFAType_name) | ||
| return trace.Wrap(err, "failed to decode require mfa type") | ||
| } |
There was a problem hiding this comment.
I think I prefer the more explicit implementation than farming things out to this helper. The call site here is quite dense and a bit hard to grok what is going on without jumping to the decodeEnum function.
There was a problem hiding this comment.
Fair enough. My thinking here is that I like the readability of the helper's usage - you can see at a glance what values map to what enum without having to parse through the type switches, bool option handling, etc. But I'm not married to the change at all, I can just close this one.
Edit: it also did turn out more complex than I envisioned, so I understand the confusion concern.
| // decodeEnum decodes a protobuf enum from a representational value, usually a bool, | ||
| // string, or from the actual enum (int32) value. If the value is valid, it is saved | ||
| // in the given enum pointer. | ||
| func decodeEnum[T ~int32](p *T, val any, representationMap map[any]T, enumMap map[int32]string) error { |
There was a problem hiding this comment.
General use helper functions like this should be covered by tests.
2 participants
Add a helper for decoding protobuf enums from their valid representions, e.g. bool, string, or number.
This is intended to make the code a bit more readable around
decodeEnumand more reusable.Prereq for #47233