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

Enable linting of CMake files using pre-commit #9484

Merged
merged 9 commits into from
Oct 29, 2021

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Oct 20, 2021

This PR resolves #9396, making it easy to run cmake-format and cmake-lint on CMake files in cudf using pre-commit. The one additional complexity associated with these hooks compared to our other pre-commit hooks is the need for the cmake-format-rapids-cmake.json configuration file that is maintained in rapids-cmake and is only added during the build process. We don't want pre-commit to fail outright when the file isn't present, so this PR introduces a custom script wrapping the cmake-format and cmake-lint hooks in a check for the presence of the file. The script respects an associated environment variable that advanced users can exploit if they store this file in a nonstandard location.

Once this PR is finalized I'll make a follow-up PR that actually applies the formatting to our existing CMake.

@vyasr vyasr added feature request New feature or request 3 - Ready for Review Ready for review by team code quality CMake CMake build issue non-breaking Non-breaking change labels Oct 20, 2021
@vyasr vyasr self-assigned this Oct 20, 2021
@vyasr vyasr requested a review from a team as a code owner October 20, 2021 23:16
@github-actions github-actions bot added libcudf Affects libcudf (C++/CUDA) code. and removed CMake CMake build issue labels Oct 20, 2021
@codecov
Copy link

codecov bot commented Oct 21, 2021

Codecov Report

Merging #9484 (1b6f509) into branch-21.12 (ab4bfaa) will decrease coverage by 0.12%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##           branch-21.12    #9484      +/-   ##
================================================
- Coverage         10.79%   10.66%   -0.13%     
================================================
  Files               116      117       +1     
  Lines             18869    19726     +857     
================================================
+ Hits               2036     2104      +68     
- Misses            16833    17622     +789     
Impacted Files Coverage Δ
python/dask_cudf/dask_cudf/sorting.py 92.90% <0.00%> (-1.21%) ⬇️
python/cudf/cudf/io/csv.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/hdf.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/orc.py 0.00% <0.00%> (ø)
python/cudf/cudf/__init__.py 0.00% <0.00%> (ø)
python/cudf/cudf/_version.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/abc.py 0.00% <0.00%> (ø)
python/cudf/cudf/api/types.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/dlpack.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/frame.py 0.00% <0.00%> (ø)
... and 66 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dce38d4...1b6f509. Read the comment docs.

@vyasr
Copy link
Contributor Author

vyasr commented Oct 28, 2021

@robertmaynard I think this should be good to go now.

@vyasr
Copy link
Contributor Author

vyasr commented Oct 29, 2021

rerun tests

@vyasr
Copy link
Contributor Author

vyasr commented Oct 29, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 5cbcf49 into rapidsai:branch-21.12 Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Make CMake style check used in CI easy to run locally
4 participants