-
Notifications
You must be signed in to change notification settings - Fork 235
feat(prql-compiler): implement Target::from_str and Target::default #1750
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
Conversation
As far as trying in PRQL/prqlc-r#78, I think this is working fine. But obviously tests on these should be added here. |
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.
Thanks a lot @eitsupi — great that you're diving into the rust!
Would it be possible to add a short test in the file? Let me know if an example would be helpful.
It's surprisingly easy! We can just have something like this, in the file: #[cfg(test)]
mod test {
use insta::assert_debug_snapshot;
#[test]
fn test_target_from_str() {
assert_debug_snapshot!(Target::from_str("sql.postgres".to_string()), @"");
assert!(Target::from_str("sql.poostgres".to_string().is_err());
}
} ...then running |
Currently, the
sql.
prefix have to be removed from the target name and pass toDialect::from_str
.This PR implements
Target::from_str
and does this on theTarget::from_str
side, eliminating the need for downstream software to implement this process.Example of Use: PRQL/prqlc-r#78