You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently the C++ include files in the RAPIDS codebase are not consistently ordered and grouped.
Inconsistent ordering makes it harder to maintain the codebase, to understand the dependencies,
and to automate changes.
Using clang-format to automate the ordering and grouping of include files will not only add
consistent style, but it will ease automation of changes. For example, We are undertaking a
refactoring of RMM that will replace rmm::mr::device_memory_resource* with rmm::device_async_resource-ref everywhere in RAPIDS (not just cuDF). This requires adding an
include to MANY files across multiple RAPIDS repos. Getting the location of this include correct
everywhere is very difficult without automatic grouping of headers.
I propose to use clang-format's IncludeCategories settings to automate the ordering and
grouping of include files in the C++ codebase consistently.
Proof-of-concept PRs have been created for RMM and cuDF:
…4205)
This uses the `IncludeCategories` settings in` .clang-format` to automate include ordering and grouping and to make include ordering more consistent with the rest of RAPIDS. For discussion, see rapidsai/cudf#15063. This PR uses a similar set of header grouping categories used in that PR, adapted for cuGraph.
One purpose of this is to make it easier to automate injection of a header change with an upcoming RMM refactoring (and in the future).
Note that this PR also updates all of cugraph to use quotes ("") when includeing local *source* headers. That is, whenever something is included from `cugraph/src/*`, it should be included with quotes rather than angle brackets. This makes it much easier to order includes from "nearest to farthest", and matches the approach now taken in other RAPIDS repos. Without switching to quotes, keeping these includes at the top requires special casing each subdirectory of `src` in the clang-format group regular expressions.
Includes from the `include` directory stay as angle brackets, because they always use `#include <cugraph/...>`.
cuGraph tests include a LOT of internal source headers. I think we should generally avoid this, but it depends on the testing philosophy. If one believes that tests should only cover the external interfaces of the library, then there is no need to include internal headers. But if we want to test internal functionality, then the tests need to be more "internal" to the library.
The header reordering in this PR also uncovered some places where headers were not included where they are used, which I have fixed.
Closes#4185
Authors:
- Mark Harris (https://github.com/harrism)
Approvers:
- Seunghwa Kang (https://github.com/seunghwak)
- Naim (https://github.com/naimnv)
- Chuck Hastings (https://github.com/ChuckHastings)
URL: #4205
Currently the C++ include files in the RAPIDS codebase are not consistently ordered and grouped.
Inconsistent ordering makes it harder to maintain the codebase, to understand the dependencies,
and to automate changes.
Using clang-format to automate the ordering and grouping of include files will not only add
consistent style, but it will ease automation of changes. For example, We are undertaking a
refactoring of RMM that will replace
rmm::mr::device_memory_resource*
withrmm::device_async_resource-ref
everywhere in RAPIDS (not just cuDF). This requires adding aninclude to MANY files across multiple RAPIDS repos. Getting the location of this include correct
everywhere is very difficult without automatic grouping of headers.
I propose to use clang-format's
IncludeCategories
settings to automate the ordering andgrouping of include files in the C++ codebase consistently.
Proof-of-concept PRs have been created for RMM and cuDF:
The ordering used for cuDF is given by the following settings.
The text was updated successfully, but these errors were encountered: