-
Notifications
You must be signed in to change notification settings - Fork 252
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
Make config wrap validators #1341
base: main
Are you sure you want to change the base?
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.
Just some general ideas - I think we should try to move forward here!
type Error = PyErr; | ||
fn try_from(value: Bound<'_, PyDict>) -> Result<Self, Self::Error> { | ||
Ok(CoreConfig { | ||
title: value.get_item("title")?.map(|v| v.extract()).transpose()?, |
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.
Maybe we use a helper function to reduce the repetition with the ?.map(|v| v.extract()).transpose()?
?
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.
Maybe a helper function and a macro would be a good combo here
} | ||
|
||
#[derive(Debug, Clone)] | ||
pub enum CacheStrings { |
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.
Nice to see all of these moved to an intuitive place
Initial stab at #1337.
Lots more work needed