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.

Sign up for GitHub

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

Jump to bottom

CMake: Formatting with cmake-format #520

Merged
merged 18 commits into from
Jul 9, 2024
Merged

CMake: Formatting with cmake-format #520

merged 18 commits into from
Jul 9, 2024

Conversation

gardner48
Copy link
Member

@gardner48 gardner48 commented Jun 21, 2024 •
edited
Loading

Add CMake formatter

@gardner48 gardner48 changed the title Cmake: formatting CMake: formatting Jun 21, 2024
@gardner48 gardner48 changed the title CMake: formatting CMake: Formatting with cmake-format Jun 21, 2024
@gardner48 gardner48 mentioned this pull request Jun 21, 2024
Closed
@gardner48
Copy link
Member Author

gardner48 commented Jun 21, 2024 •
edited
Loading

Also see alternative formatter in #521

@balos1
Copy link
Member

balos1 commented Jun 22, 2024

I have a slight preference to the formatting in this PR over #521. Also, cmake-format seems to be much more popular so I think its probably the better choice for that reason too.

@gardner48
Copy link
Member Author

gardner48 commented Jun 22, 2024 •
edited
Loading

I have a slight preference to the formatting in this PR over #521. Also, cmake-format seems to be much more popular so I think its probably the better choice for that reason too.

My main concern with cmake-format was that it doesn't seem to have any activity in the past three years. Which is why I looked at gersemi as well. One thing I like about gersemi over cmake-format is it can format custom functions/macros without having to supply additional information so it does a better job with things like with sundials_add_library.

@gardner48
Copy link
Member Author

I updated the cmake-format.py file so it knows about our custom macros so those are formatted better now

@gardner48 gardner48 marked this pull request as ready for review June 30, 2024 02:25
@gardner48
update docs
fede229
@gardner48
fix typo
3843c5f
@gardner48
Merge branch 'develop' into cmake/formatting
52b02e8 
Conflicts:
  .github/workflows/check-format.yml
  doc/superbuild/source/developers/style_guide/SourceCode.rst
  scripts/format.sh
@balos1
Copy link
Member

balos1 commented Jul 1, 2024

I have a slight preference to the formatting in this PR over #521. Also, cmake-format seems to be much more popular so I think its probably the better choice for that reason too.

My main concern with cmake-format was that it doesn't seem to have any activity in the past three years. Which is why I looked at gersemi as well. One thing I like about gersemi over cmake-format is it can format custom functions/macros without having to supply additional information so it does a better job with things like with sundials_add_library.

Hmm, I did not realize cmake-format was not maintained anymore. If it starts causing problems for us later on, I suppose worst case scenario we disable it in the formatting check.

@gardner48 gardner48 merged commit c6b9a02 into develop Jul 9, 2024
24 checks passed
@gardner48 gardner48 deleted the cmake/formatting branch July 9, 2024 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@balos1 balos1 balos1 approved these changes

@cswoodward cswoodward Awaiting requested review from cswoodward cswoodward is a code owner

@drreynolds drreynolds Awaiting requested review from drreynolds drreynolds is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gardner48 @balos1