-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
GH-36674: [C++] Use anonymous namespace in arrow/ipc/reader.cc #36937
Conversation
trying to fix #36674 |
hi, @westonpace , Would you like to review this PR when anytime you are available? Regards, |
@pegasas Can you make the PR description a bit more explicit? |
Sure. I added more description into that. |
|
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 1a00fec. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them. |
…pache#36937) ### Rationale for this change The following types and functions should be in file scope: IpcReadConext, BatchDataReadRequest, ArrayLoader, DecompressBuffer(s), LoadRecordBatch*, GetCompression*, ReadRecordBatchInternal, GetInclusionMaskAndOutSchmea, UnpackSchemaMessage, ReadDictionary, AsyncRecordBatchStreamReaderImpl. ### What changes are included in this PR? Use anonymous namespace around the aforementioned definitions. ### Are these changes tested? No ### Are there any user-facing changes? No Lead-authored-by: pegasas <[email protected]> Co-authored-by: Antoine Pitrou <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
Rationale for this change
The following types and functions should be in file scope:
IpcReadConext, BatchDataReadRequest, ArrayLoader,
DecompressBuffer(s), LoadRecordBatch*, GetCompression*,
ReadRecordBatchInternal,
GetInclusionMaskAndOutSchmea, UnpackSchemaMessage,
ReadDictionary, AsyncRecordBatchStreamReaderImpl.
What changes are included in this PR?
Use anonymous namespace around the aforementioned definitions.
Are these changes tested?
No
Are there any user-facing changes?
No