-
Notifications
You must be signed in to change notification settings - Fork 2
Add linter analysis workflow #9
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
base: main
Are you sure you want to change the base?
Conversation
- name: Fail fast | ||
if: steps.linter.outputs.checks-failed > 0 | ||
run: exit 1 |
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.
Should we keep this (and force fixing everything, or skip lines if we consider the errors/warnings as false positives) or remove it to only have annotated files as a reference?
@@ -12,10 +12,12 @@ | |||
|
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.
Regarding this error:
src/serialize.cpp:8:10 [clang-diagnostic-error]
'Message_generated.h' file not found
We would probably need to link flatbuffers_interface
as public to the target to fix it.
We can also choose to ignore this and keep the private link if we hold on to it.
|
||
// Calculate the size of the validity and data buffers | ||
int64_t validity_size = (arrow_arr.length + 7) / 8; | ||
int64_t validity_size = (arrow_arr.length + arrow_alignment - 1) / arrow_alignment; |
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.
False positive?
src/serialize.cpp:157:17 [cppcoreguidelines-init-variables]
variable 'validity_size' is not initialized
dst += sizeof(uint32_t); | ||
// Copy the RecordBatch metadata into the buffer | ||
memcpy(dst, batch_builder.GetBufferPointer(), batch_meta_len); | ||
// Add padding to align the body to an 8-byte boundary | ||
memset(dst + batch_meta_len, 0, aligned_batch_meta_len - batch_meta_len); | ||
if (static_cast<size_t>(aligned_batch_meta_len) >= batch_meta_len) |
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.
src/serialize.cpp:200:13 [modernize-use-integer-sign-comparison]
comparison between 'signed' and 'unsigned' integers
Still complaining even after the cast...
The thing is that we're kind of constrained on some of the functions from the generated headers in arrow/flatbuffers, thus mixing up unsigned and signed integers in general in this codebase...
No description provided.