Conversation
myrrc
force-pushed
the
myrrc/scan-api-docs
branch
from
6284347 to
8e70414
Compare
myrrc
force-pushed
the
myrrc/scan-api-docs
branch
from
e675e19 to
f143c23
Compare
myrrc
force-pushed
the
myrrc/scan-api-docs
branch
2 times, most recently
from
2ebedaf to
ba77e1f
Compare
myrrc
force-pushed
the
myrrc/scan-api-docs
branch
from
ba77e1f to
015ff48
Compare
|
@claude in depth review |
|
Claude review automation is disabled for pull requests that modify Why:
Ask a human reviewer to inspect workflow changes directly. |
| } | ||
| } | ||
|
|
||
| int main(int argc, char *argv[]) { |
There was a problem hiding this comment.
Add explicit return 0 for C main.
| return array; | ||
| } | ||
|
|
||
| int main(int argc, char *argv[]) { |
There was a problem hiding this comment.
Add explicit return 0 for C main.
| VX_CARD_ESTIMATE = 1, | ||
| VX_CARD_MAXIMUM = 2, | ||
| } vx_cardinality; | ||
| /** |
There was a problem hiding this comment.
Should we add a diagnostic to the header that this API is unstable? I guess in the end we'd want ABI stability here. We don't have to address this in this PR but keep in mind that basically anything that lands in Vortex, will be tried out by others so it's good to be explicit on what's stable and what's unstable.
| } | ||
| None => { | ||
| rc.cardinality = vx_cardinality::VX_CARD_UNKNOWN; | ||
| rc.r#type = vx_estimate_type::VX_ESTIMATE_UNKNOWN; |
There was a problem hiding this comment.
This requires the call site to init the row count which not all call sites in this PR do.
| // Caller can use partition estimates to schedule worker threads. | ||
| print_estimate("Partition count", &partition_estimate); | ||
| if (threads == 0 && partition_estimate.type != VX_ESTIMATE_UNKNOWN) { | ||
| threads = partition_estimate.estimate; |
There was a problem hiding this comment.
If the user didn't pass a thread count and the estimate is unknown the thread count stays 0.
| const vx_array *height_array = | ||
| vx_array_new_primitive(PTYPE_U16, height_buffer, SAMPLE_ROWS, &validity, &error); | ||
| if (error != NULL) { | ||
| print_error("Error adding age array field to root array", error); |
|
|
||
| vx_error *error = NULL; | ||
| const vx_array *age_array = vx_array_new_primitive(PTYPE_U8, age_buffer, SAMPLE_ROWS, &validity, &error); | ||
| if (error != NULL) { |
There was a problem hiding this comment.
Error handling looks redundant in this fn, could extract into a separate fn.
| } | ||
|
|
||
| struct scan_thread_info { | ||
| pthread_t thread_id; |
There was a problem hiding this comment.
We're using pthread_t here but later assign ints.
2 participants
Uh oh!
There was an error while loading. Please reload this page.