-
Notifications
You must be signed in to change notification settings - Fork 234
Conversation
jbkoh
commented
Dec 9, 2024
just added a test too.
jbkoh
commented
Dec 9, 2024
@Gobot1234 please review and let me know if it's in the right direction.
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.
Having a function for each of these adds a lot of bulk to the generated files, can this not go in the original definition for Enum and also be inlined into a lambda as this doesn't need type hints?
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 fine with the suggestion. Where do you recommend to declare the validate function? We can also define it as a method in betterproto library too.
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.
something like
def get_validate_betterproto_enum(cls): def validate(cls, value): return cls(value) return validate
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.
@Gobot1234 I'm not sure if I understood your suggestion properly actually. At least, I changed to use lambda instead.
Another option is just to define this validator as a class method for class Enum by default.
Please elaborate your suggestion again and I'd be happy to update my PR. Thanks!
Uh oh!
There was an error while loading. Please reload this page.
Summary
Checklist