-
Notifications
You must be signed in to change notification settings - Fork 915
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
Cleanup ORC chunked writer #13091
Cleanup ORC chunked writer #13091
Conversation
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 improved consistency is good, but isn't the leading underscore a bit redundant in C++ where the private access specifier already conveys that these are internal variables?
Consider the situations like this:
vs this:
In the first situation, you can immediately recognize that we are changing a local variable by applying The underscore added to member variables is for the sheer clearness to help working on the code better, not to mean for any correctness. |
/merge |
Similar to #13091, this changes the internal variables of Parquet chunked writer: * Renaming them to have a `_` prefix consistently. * Add `const` qualifier to some variables that are writer parameters. * Regroup them. There is not any new implementation added. However, the unused parameter `mr` is removed from its interface thus this is flagged as breaking changes. Closes: * #13079 Authors: - Nghia Truong (https://github.com/ttnghia) Approvers: - Mike Wilson (https://github.com/hyperbolic2346) - Karthikeyan (https://github.com/karthikeyann) URL: #13094
This changes the internal variables of ORC chunked writer:
_
prefix consistently.const
qualifier to some variables that are writer parameters.There is not any new implementation added. However, the unused parameter
mr
is removed from its interface thus this is flagged asbreaking
changes.Closes:
cudf::io::detail::orc::writer::impl
#12973