Automate C++ include file grouping and ordering using clang-format #1349
Conversation
harrism
added
improvement
…os (doesn't affect any files currently)
|
+1 to the new copyright update script, we're planning to eventually roll that out to all RAPIDS repos. |
| Priority: 3 | ||
| - Regex: '^<(cuspatial|cuproj)/' # cuSpatial/cuProj includes | ||
| Priority: 4 | ||
| - Regex: '^<(cudf_test|cudf|cuml|raft|kvikio)' # Other RAPIDS includes |
There was a problem hiding this comment.
I thought about removing cuml/raft/kvikio since they are not used, should it?
There was a problem hiding this comment.
We could, but I'm also trying to keep things relatively consistent across RAPIDS repos. But I'm not confident in that direction. Thoughts?
There was a problem hiding this comment.
Yeah I'm not confident either - especially if we apply this remove-not-used rule, then what about the cuda libraries below? If we remove cuda rules, are we sure that future contributors do not use them?
From that end, I think the future-facing approach is to maintain the disjunction set from all RAPIDS libraries to embed the collective behavioral customs. This reduces possible conflicts when migrating code or merging repos.
|
/merge |
Description
This uses the IncludeCategories settings in .clang-format to attempt to enforce our documented #include order in libcuspatial. For discussion, see rapidsai/cudf#15063. This PR uses a subset of the header grouping categories used in that PR.
Note that this also updates the pre-commit configuration to check and correct file copyright years. This comes from rapidsai/cudf#14917. @KyleFromNVIDIA can you confirm whether or not it's OK to do this? It seems to be working fine.
Closes #1345
Checklist