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

Proposed clang-format Config #184

Merged

Conversation

insertinterestingnamehere
Copy link
Collaborator

I'd like to start using clang-format to manage formatting. As far as I can tell, the formatting isn't especially consistent across the codebase right now. Here's an initial config based on my own preferences that we can use as a starting point. Feel free to suggest any changes.

I'll leave this open for a while to give time for any comments. Once we've settled on a format spec to use, I'll set things up to exclude any vendored files, apply the format to the whole codebase whenever there aren't open pull requests, then set up a CI build to check that the formatting has been correctly applied to each pull request.

@janciesko
Copy link
Collaborator

I am ok using clang-format.

@janciesko
Copy link
Collaborator

You can also reuse this config file, for consistency across code bases. https://github.com/kokkos/kokkos/blob/master/.clang-format

@insertinterestingnamehere
Copy link
Collaborator Author

Yah, I'm fine with mimicking their config too if that's preferable. I don't think it's quite possible to maintain perfect reuse because of options that tag the behavior of specific macros. AttributeMacros is the only one like that that we'll currently want to use, but others could really easily become needed too (for example for warning suppression macros). We can do things mostly the same though.

The main nontrivial differences between my proposed file and theirs is that they set AlignConsecutiveAssignments: true, that I enforce header alphabetizing inside blocks, and that I set QualifierAlignment to follow the "east const" convention instead of leaving it inconsistent. We can also enforce "west const" instead if we want.

There are also some places where the default BreakArrays setting will make some things look a little silly, but they're fairly rare.

I think InsertNewlineAtEOF: true can make diffs slightly less cluttered because of some weirdness with how git handles newlines on Windows and also different editor configs, but I haven't confirmed that.

@janciesko janciesko self-requested a review November 29, 2023 18:44
Copy link
Collaborator

@janciesko janciesko left a comment

Choose a reason for hiding this comment

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

LGTM

@insertinterestingnamehere insertinterestingnamehere merged commit e71f534 into sandialabs:main Nov 30, 2023
116 of 230 checks passed
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.

None yet

2 participants