-
Notifications
You must be signed in to change notification settings - Fork 42
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
refactor: move parser into own crate #2747
Conversation
…glaredb into universalmind303/extract-parser
some more prereq work for making the tableoptions more flexible as part of #2744. The `Arbitrary` trait has a cascading effect, so it's kind of all or nothing, and since we can't have `trait TableOptions: Arbitrary` in an object-safe way, it's gotta go.
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.
This seems my only question/concern is "should we worry about the proptest or think about replacing it?"
I don't know if there's anything to be done about it, but the name collision between our parser
and the upstream parser package (particularly ParserError
) seems like a thing that will be confusing in the future.
@@ -25,6 +25,7 @@ futures = { workspace = true } | |||
gcp-bigquery-client = "0.18.0" | |||
klickhouse = { version = "0.11.2", features = ["tls"] } | |||
protogen = { path = "../protogen" } | |||
parser = {path = "../parser"} |
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.
super-nit: we added spaces in other places for this form.
idle musing: is there a toml linter we could get?
some prereq work for #2744