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

ARROW-4918: [C++] Add cmake-format to pre-commit #3935

Closed
wants to merge 1 commit into from

Conversation

xhochy
Copy link
Member

@xhochy xhochy commented Mar 16, 2019

Also extended the pattern to cover all the CMake code I've written in the last weeks.

@fsaintjacques
Copy link
Contributor

LGTM

@wesm
Copy link
Member

wesm commented Mar 16, 2019

Hm, do we want to reformat all the cmake_modules/* files? Some of these have code reused from other projects and so it could just cause unnecessary divergence. Definitely we should keep ThirdpartyToolchain.cmake clean

@xhochy
Copy link
Member Author

xhochy commented Mar 17, 2019

Hm, do we want to reformat all the cmake_modules/* files? Some of these have code reused from other projects and so it could just cause unnecessary divergence. Definitely we should keep ThirdpartyToolchain.cmake clean

Changed this to an explicit list and reverted the changes to the files that were imported.

@codecov-io
Copy link

codecov-io commented Mar 17, 2019

Codecov Report

Merging #3935 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3935      +/-   ##
==========================================
- Coverage   87.75%   87.74%   -0.01%     
==========================================
  Files         727      727              
  Lines       89507    89507              
  Branches     1252     1252              
==========================================
- Hits        78547    78542       -5     
- Misses      10842    10847       +5     
  Partials      118      118
Impacted Files Coverage Δ
js/src/ipc/metadata/json.ts 92.39% <0%> (-4.35%) ⬇️
js/src/ipc/metadata/message.ts 92.99% <0%> (-1.28%) ⬇️
js/src/schema.ts 88.77% <0%> (-1.03%) ⬇️
cpp/src/plasma/store.cc 91.5% <0%> (+0.33%) ⬆️
cpp/src/plasma/thirdparty/ae/ae.c 71.69% <0%> (+0.94%) ⬆️

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 9d2280f...0c8db42. Read the comment docs.

Copy link
Member

@wesm wesm left a comment

Choose a reason for hiding this comment

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

+1

@wesm wesm closed this in 79c93c7 Mar 17, 2019
@wesm wesm deleted the ARROW-4918 branch March 17, 2019 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants