Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: create an exportable version of plan loading #91
feat: create an exportable version of plan loading #91
Changes from 32 commits
0ea3c42
6758f3f
af4fb1e
bfe2ff6
44188b6
20acf0b
77fcd30
df1b42c
58bed47
73bec1f
74bf673
80b2c3e
347fbe2
e4485db
a72a827
d0c187d
320053c
125a8cb
db12b3d
12b54fe
3d1e40e
e49c11a
00de8bc
e754a1c
1b191ea
f5ce243
755c4ef
c28f6f0
dac8ea9
5c3d5f2
dd6b5c3
8fc79fb
7f8a7da
fbe9231
e487313
8ea2556
0d0246d
06cd5c9
9be359b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Why not? Does this mean you can't build the planloader in debug mode (annoying but ok)? Or does this mean that someone linking to the planloader cannot build in debug (probably an issue)?
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.
The problem is all of the sanitization stuff that we're including in debug mode. An unfettered debug mode would be fine.
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've become used to
//
indicating "internal comment" and///
indicating "formal documentation". Do we want to stick to that convention?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 not sure I want to formalize this interface. If someone wants to use the C++ code they should be using the recently exposed substrait_io library. This version is for wrapping purposes only so I'd like it to be more obscure.
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.