Skip to content
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

Use east const in include files #13494

Merged
merged 8 commits into from
Jun 14, 2023

Conversation

karthikeyann
Copy link
Contributor

@karthikeyann karthikeyann commented Jun 1, 2023

Description

Use east const in cpp/include/ files
used clang-format QualifierAlignment: Right to do the clean up.

since https://clang.llvm.org/docs/ClangFormatStyleOptions.html#qualifieralignment has warning that
Setting QualifierAlignment to something other than Leave, COULD lead to incorrect code formatting due to incorrect decisions made due to clang-formats lack of complete semantic information. As such extra care should be taken to review code changes made by the use of this option.
So it's not added to .clang-format

Split to multiple PRs for easy reviewing #13491, #13492, #13493, #13494

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@karthikeyann karthikeyann added 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. tech debt improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jun 1, 2023
@karthikeyann karthikeyann requested a review from a team as a code owner June 1, 2023 18:05
@karthikeyann karthikeyann self-assigned this Jun 1, 2023
@karthikeyann karthikeyann requested review from vyasr and PointKernel June 1, 2023 18:05
Copy link
Member

@PointKernel PointKernel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Good to know that clang-format can automate this kind of cleanup.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR but this should be a cuh file instead of c-style h file.

rapids-bot bot pushed a commit that referenced this pull request Jun 6, 2023
Use east const in _cpp/tests/_ files
used clang-format `QualifierAlignment: Right` to do the clean up.

since https://clang.llvm.org/docs/ClangFormatStyleOptions.html#qualifieralignment has warning that 
`Setting QualifierAlignment to something other than Leave, COULD lead to incorrect code formatting due to incorrect decisions made due to clang-formats lack of complete semantic information. As such extra care should be taken to review code changes made by the use of this option.`
So it's not added to .clang-format

Split to multiple PRs for easy reviewing #13491, #13492, #13493, #13494

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - Mark Harris (https://github.com/harrism)

URL: #13492
rapids-bot bot pushed a commit that referenced this pull request Jun 10, 2023
Use east const in _cpp/src/_ files
used clang-format `QualifierAlignment: Right` to do the clean up.

since https://clang.llvm.org/docs/ClangFormatStyleOptions.html#qualifieralignment has warning that 
`Setting QualifierAlignment to something other than Leave, COULD lead to incorrect code formatting due to incorrect decisions made due to clang-formats lack of complete semantic information. As such extra care should be taken to review code changes made by the use of this option.`
So it's not added to .clang-format

Split to multiple PRs for easy reviewing #13491, #13492, #13493, #13494

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - Vukasin Milovanovic (https://github.com/vuule)

URL: #13493
@karthikeyann karthikeyann changed the title Use east const in include files Use east const in include files Jun 12, 2023
@karthikeyann karthikeyann changed the title Use east const in include files Use east const in include files Jun 13, 2023
@karthikeyann karthikeyann changed the title Use east const in include files Use east const in include files Jun 13, 2023
@karthikeyann karthikeyann changed the title Use east const in include files Use east const in include files. Jun 13, 2023
@karthikeyann karthikeyann changed the title Use east const in include files. Use east const in include files Jun 13, 2023
@karthikeyann karthikeyann added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Jun 13, 2023
@karthikeyann
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit c929a84 into rapidsai:branch-23.08 Jun 14, 2023
rapids-bot bot pushed a commit that referenced this pull request Jul 13, 2023
Use east const in _cpp/benchmarks/_ files
used clang-format `QualifierAlignment: Right` to do the clean up.

since https://clang.llvm.org/docs/ClangFormatStyleOptions.html#qualifieralignment has warning that 
`Setting QualifierAlignment to something other than Leave, COULD lead to incorrect code formatting due to incorrect decisions made due to clang-formats lack of complete semantic information. As such extra care should be taken to review code changes made by the use of this option.`
So it's not added to .clang-format

Split to multiple PRs for easy reviewing #13491, #13492, #13493, #13494

Authors:
  - Karthikeyan (https://github.com/karthikeyann)
  - Nghia Truong (https://github.com/ttnghia)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - Bradley Dice (https://github.com/bdice)

URL: #13491
rapids-bot bot pushed a commit that referenced this pull request Jan 23, 2024
This PR documents the libcudf preference for "east const." Follow-up from #13491, #13492, #13493, #13494.

Authors:
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - David Wendt (https://github.com/davidwendt)
  - Nghia Truong (https://github.com/ttnghia)
  - Yunsong Wang (https://github.com/PointKernel)

URL: #14836
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants